mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
d24a79f741924dd797d0c2935829af0a8937c777
3594
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Devin Foley
|
d24a79f741 |
ci(dependabot): surface major npm updates as one grouped weekly PR (#11307)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Dependabot keeps the npm dependency tree and the GitHub Actions workflows current with weekly update PRs > - The npm config ignores every major version bump with a wildcard `ignore` rule, and no other process reports pending majors > - Major-version debt grows silently, and ignore rules also suppress Dependabot security updates when the fix ships only in a newer major > - Individual major PRs are not a good replacement: the board decided in #7560 to keep the PR list mergeable, and a flood of breaking bumps works against that > - This pull request removes the blanket ignore and groups all pending majors into one weekly PR, while minors and patches keep one PR per bump > - The benefit is a standing, visible signal of pending major updates, at a cost of at most one extra PR per week ## Linked Issues or Issue Description **What existing behavior does this improve?** The Dependabot npm update flow configured in `.github/dependabot.yml`. **Current behavior** Dependabot opens weekly PRs for minor and patch npm updates. A wildcard `ignore` rule suppresses every major version update. No report or reminder replaces the suppressed PRs — the comment says "review those manually", but nothing triggers that review. Ignore rules also apply to Dependabot security updates, so a security fix that ships only in a newer major is suppressed as well. **Proposed behavior** Dependabot opens one grouped weekly PR that contains every pending major npm update. Minor and patch updates keep their current one-PR-per-bump flow. A deliberate hold on a specific major can use a targeted per-dependency `ignore` entry instead of the wildcard. **Reason and benefit** Silent major-version drift compounds: each skipped major makes the eventual upgrade jump larger and riskier, especially across peer-dependency families. A single grouped PR makes the backlog visible in the PR list without flooding it. When the grouped PR is green, it merges cheaply. When it is red, it is a visible standing task instead of invisible debt. **Breaking changes** None. This changes repository automation only. Runtime behavior, response shapes, and outputs are unchanged. **Additional context** Related history: #7483 grouped patch/minor updates by dependency type, and #7560 reverted that grouping because the resulting 26-package PR was hard to merge. This PR does not touch the patch/minor flow. It only groups majors, which currently produce no PRs at all — it adds a signal that does not exist today rather than replacing individually mergeable PRs. ## What Changed - Removed the wildcard `ignore` rule for `version-update:semver-major` from the npm ecosystem in `.github/dependabot.yml`. - Added a `major-updates` group (`applies-to: version-updates`, `update-types: ["major"]`, `patterns: ["*"]`) so all pending majors land in one weekly grouped PR. - Left the schedule, labels, PR limits, and the github-actions ecosystem unchanged. ## Verification - `npx js-yaml .github/dependabot.yml` parses cleanly and the `groups` stanza follows the Dependabot v2 schema (`applies-to`, `update-types`, `patterns`). - After merge: check Insights → Dependency graph → Dependabot for config errors. The next weekly run (Monday 06:00) opens a single `major-updates` grouped PR that lists the pending majors. - No code changed, so the test suite is unaffected. ## Risks - Low risk. This is CI/automation configuration only. - The first grouped PR may be large, and red if several majors break the build. That is the intended visibility mechanism, and it does not block other work. A noisy or deliberately held-back dependency can be excluded from the group with `exclude-patterns` or a targeted per-dependency `ignore` entry. - This does not regroup minors or patches, so it does not reintroduce what #7560 reverted. ## Model Used - Claude Fable 5 (Anthropic), model ID `claude-fable-5`, via Claude Code CLI, extended thinking and tool use 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 - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [ ] I have run tests locally and they pass — N/A, YAML-only CI config change; validated with `js-yaml` - [ ] I have added or updated tests where applicable — N/A, no code changed - [x] I have updated relevant documentation to reflect my changes — none reference the Dependabot config - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green — one e2e shard flaked on an unrelated MCP UI spec and passed on re-run with identical code - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting mergecanary/v2026.813.0-canary.0 |
||
|
|
a8d118a779 |
Prefer public base URL for generated invite links (#7619)
Fixes #7623 ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Company invites are part of the access subsystem and must produce URLs that recipients can open from outside the host machine. > - Paperclip already has public/auth base URL configuration for deployments behind a public hostname, Tailscale, or a reverse proxy. > - Invite URL composition was still deriving its origin from the incoming request host, so loopback-bound servers emitted `http://127.0.0.1:3100/invite/...`. > - A loopback invite URL is not shareable with a remote human or agent, even when the token itself is valid. > - This pull request makes invite URL builders prefer the configured public base URL and keep the existing request-host fallback when it is unset. > - The benefit is that copied invite links use the reachable deployment origin without changing local-only behavior. ## Linked Issues or Issue Description Fixes #7623 No duplicate or related PRs/issues were found in a GitHub search for invite URL, loopback, public base URL, and `authPublicBaseUrl` terms. ## What Changed - Added base URL resolution in `server/src/routes/access.ts` that strips trailing slashes and prefers configured `authPublicBaseUrl` over the request-derived host. - Threaded `authPublicBaseUrl` through invite summary, invite onboarding manifest, onboarding text, access routes, `createApp`, and server startup wiring. - Added `server/src/__tests__/invite-url-public-base-url.test.ts` covering configured public-base precedence, unset fallback behavior, and trailing-slash normalization. - Registered the invite public-base URL test in the serialized Vitest server runner. ## Verification ```bash pnpm install --frozen-lockfile pnpm exec vitest run server/src/__tests__/invite-url-public-base-url.test.ts pnpm run test:run:serialized ``` Local results from the rebased PR branch: - `pnpm install --frozen-lockfile` exited 0. - Targeted invite URL test exited 0: 1 file, 3 tests passed. - Serialized server suite exited 0: 106 serialized suites completed; the new invite URL test passed inside that runner. Manual check after deployment: set `PAPERCLIP_AUTH_PUBLIC_BASE_URL` or equivalent public base URL config, create a company invite, and confirm the returned/copied invite URL uses that public origin instead of `127.0.0.1`. ## Risks Low risk. The new public base URL parameter is optional and falls back to existing request-derived behavior when unset. The main operational risk is misconfigured public base URL input; the implementation only trims trailing slashes and otherwise trusts the configured origin. ## Model Used - Original implementation: Anthropic `claude-sonnet-4-6`, 200k context, tool use and test execution. - Conflict repair and verification: OpenAI Codex GPT-5.5, coding agent with shell, git, GitHub CLI, 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 searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [ ] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Coder (Claude) <coder-claude@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Paperclip Coder (Claude) <lad-agent@paperclip.ing>canary/v2026.812.0-canary.16 |
||
|
Dylan RoyandPaperclip
|
6a546e8a9a |
fix(server): align agent run JWT default TTL with documented 48h default (#10176)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Local adapters (claude_local, codex_local) run agent heartbeats as
child processes, with a short-lived run JWT injected as
`PAPERCLIP_API_KEY` at spawn time
> - That JWT is minted exactly once, when the adapter spawns the process
— its TTL must therefore cover the entire wall-clock life of the run,
not just a prompt startup
> - On laptops the gap between spawn and first real execution can be
huge: a timer heartbeat scheduled while the lid is closed fires during a
~2s macOS dark wake, the machine re-sleeps immediately, and the frozen
child only executes during a later, longer wake — over an hour of
wall-clock delay in observed runs
> - The server's default TTL was 1h, so those sessions started with an
already-expired `PAPERCLIP_API_KEY` and every control-plane call 401'd;
the agent had to recover by manually minting a fresh key
> - The 1h default was also a spec drift: the CLI `env` command
(`DEFAULT_AGENT_JWT_TTL_SECONDS`) and the agent-authentication design
doc both document 172800s (48h)
> - This pull request realigns the server default to 48h and documents
the host-suspension constraint at the mint site and in the regression
test
> - The benefit is that lid-closed/suspended-host heartbeat runs come up
with a valid credential, and the three places that state the default now
agree
## Linked Issues or Issue Description
No public GitHub issue exists for this; per the bug-report template:
- **What happened:** A timer-driven heartbeat run on a MacBook (lid
closed, on battery) was invoked during a ~2s dark wake. The adapter
spawned the CLI and logged init within 2s, then the host re-slept and
the session sat frozen for ~64 minutes until a longer dark wake let it
execute. By then the injected run JWT (1h TTL, minted at spawn) had
expired, so every API call from the agent returned 401 and the run could
only recover via a manually minted key. A second agent's run the same
night showed the identical signature (output timestamps exactly matching
`pmset -g log` dark-wake windows).
- **Expected behavior:** A run that starts late because the host was
suspended should still have a valid `PAPERCLIP_API_KEY` when it finally
executes.
- **Steps to reproduce:** Run Paperclip on a laptop with a
`claude_local` agent on a timer heartbeat; close the lid on battery
overnight; observe a run invoked during a dark wake whose session
executes >1h later with an expired token (compare run-log timestamps to
`pmset -g log` sleep/wake entries).
- **Version/commit:** current `master` (
|
||
|
|
b7b8fbf688 |
fix(adapter-utils): let explicit PAPERCLIP_API_URL override the derived runtime URL in run env (#10339)
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - Every agent run gets a run-scoped bridge into the Paperclip API
through the injected `PAPERCLIP_API_URL` / `PAPERCLIP_API_KEY` env vars,
built by `buildPaperclipEnv` in
`packages/adapter-utils/src/server-utils.ts`
> - `buildPaperclipEnv` resolves that URL as `PAPERCLIP_RUNTIME_API_URL
?? PAPERCLIP_API_URL ?? http://<listen-host>:<port>`, and the server
always exports `PAPERCLIP_RUNTIME_API_URL` derived from
`authPublicBaseUrl` at boot
> - When `authPublicBaseUrl` points at an address that is not reachable
from inside the runtime container (e.g. a VPN/tailnet-only address used
to keep the web UI off the public internet), every local run receives a
dead API URL (`curl` exit 7) and agents only survive by hand-rolling a
localhost fallback
> - An operator-set `PAPERCLIP_API_URL` is the documented escape hatch —
`docs/deploy/environment-variables.md` states the server "preserves the
value" when set externally and that the run-level var "inherits the
server-level value" — but the run env builder inverts the precedence, so
the override never actually reaches runs
> - This pull request swaps the precedence in `buildPaperclipEnv` so an
explicit `PAPERCLIP_API_URL` wins over the derived runtime URL, aligning
the behavior with the documented contract
> - The benefit is that operators with split-horizon topologies (public
auth URL != container-reachable URL) can point agent runs at a reachable
endpoint with one env var, with zero behavior change for deployments
that do not set it
## Underlying Issue
No pre-existing public issue covers this, so per CONTRIBUTING ("Link
Issues or Describe Them In-PR") here are the `bug_report.yml` fields
inline:
- **What happened:** with `PAPERCLIP_AUTH_PUBLIC_BASE_URL` on a
tailnet-only address and `PAPERCLIP_API_URL=http://localhost:3100`
explicitly set in the server environment, every agent run still received
`PAPERCLIP_API_URL=http://100.x.y.z:3100` (the derived,
container-unreachable URL); `curl` from inside the run exits 7 and
agents can only reach the API by hand-rolling a localhost fallback
- **Expected behavior:** the run env inherits the operator-configured
`PAPERCLIP_API_URL`, as documented in
`docs/deploy/environment-variables.md` ("preserves the value", run-level
var "inherits the server-level value")
- **Steps to reproduce:** (1) set `PAPERCLIP_AUTH_PUBLIC_BASE_URL` to an
address not reachable from inside the server container, (2) set
`PAPERCLIP_API_URL=http://localhost:3100` in the server env, (3) trigger
any agent run and inspect the spawned process env: it carries the
derived URL, not the override
- **Version/commit:** reproduced on the `91e58acb` image (2026-07-19);
the precedence is unchanged on current `master` (`a3b293e`)
- **Deployment mode:** single-host Docker Compose, local adapters
(`claude_local`/`codex_local`), web UI exposed via VPN/tailnet only
## Related PRs (dedup search)
Several in-flight PRs touch the same pain point (runs receiving an
unreachable injected API URL) — linked for reviewer context; none of
them honors the documented explicit override, and the older ones appear
stale:
- #9916 — reworks `PAPERCLIP_RUNTIME_API_URL` derivation and port
preservation (server side); complementary, does not change run-env
precedence
- #8130 — honors a pre-set `PAPERCLIP_RUNTIME_API_URL` (server side); a
complementary escape hatch via the runtime var instead of the documented
`PAPERCLIP_API_URL` override
- #8025 — heuristic: prefer loopback when the runtime bind is loopback
(no activity since Jun 12)
- #5692 — heuristic loopback-safe URL inside `buildPaperclipEnv` (no
activity since May 14)
- #4877 — broader same-host injection rework across 10 files (no
activity since May 2)
- #4794 — always forces loopback for spawned agents (no activity since
Apr 30; would break split-horizon setups where a reachable non-loopback
URL is intended)
This PR intentionally takes the Path-1 route from CONTRIBUTING: the
smallest possible change (swap two lines so the documented operator
override wins) plus regression tests, rather than a new heuristic.
## What Changed
- `packages/adapter-utils/src/server-utils.ts`: `buildPaperclipEnv` now
resolves the injected URL as `PAPERCLIP_API_URL ??
PAPERCLIP_RUNTIME_API_URL ?? http://<listen-host>:<port>` (explicit
override first), with a short comment explaining why
- `packages/adapter-utils/src/server-utils.test.ts`: three new tests
covering the override precedence, the derived-URL fallback, and the
listen-host default (including the `0.0.0.0` to `localhost` mapping)
- `server/src/__tests__/paperclip-env.test.ts`: updated the expectation
that encoded the old runtime-URL-first precedence and added the
symmetric fallback case (runtime URL used when no explicit override is
set)
- No docs changes needed: `docs/deploy/environment-variables.md` already
describes the fixed behavior
## Verification
- `vitest run` on the new `buildPaperclipEnv` tests in
`packages/adapter-utils`: 3/3 pass
- `vitest run` on `server/src/__tests__/paperclip-env.test.ts` after the
expectation update: 5/5 pass (the first CI run correctly flagged the one
test that encoded the old precedence)
- Reproduced and verified on a production deployment (single-host
Docker, `PAPERCLIP_AUTH_PUBLIC_BASE_URL` on a tailnet-only address):
- Before: freshly spawned runs received
`PAPERCLIP_API_URL=http://100.x.y.z:3100` (verified in the spawned
process `/proc/<pid>/environ`); `curl` to it from inside the container
exits 7
- After (with `PAPERCLIP_API_URL=http://localhost:3100` in the compose
environment): a fresh run received `http://localhost:3100`, and `curl
$PAPERCLIP_API_URL/api/agents/me` with the run-scoped key returned HTTP
200; the run finished `succeeded` with usage telemetry recorded
## Risks
- Low. Behavior changes only for deployments that explicitly set
`PAPERCLIP_API_URL`; when unset (the default),
`PAPERCLIP_RUNTIME_API_URL` is used exactly as before
- The sandbox callback bridge (`execution-target.ts`) is intentionally
untouched: remote sandboxes genuinely need the publicly reachable URL,
and its `input.hostApiUrl || PAPERCLIP_RUNTIME_API_URL || ...` chain
still provides it
## Model Used
- Claude Fable 5 (Anthropic, model ID `claude-fable-5`), extended
thinking + agentic tool use via Claude Code, operating over SSH against
the affected deployment
## Checklist
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [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
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Sergio-LPA <204395363+Sergio-LPA@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Philip D'Souza
|
4660562fde |
fix(opencode-local): make the model-availability probe non-fatal (#10294)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents run through adapters; the `opencode-local` adapter shells out to the OpenCode CLI and, before each run, does a pre-flight `opencode models` **availability probe** to fail fast on a misconfigured `provider/model`. > - That probe was written to **throw on any probe failure** — a timeout, a non-zero exit, or a transient `Unexpected error` from the CLI — which aborts the whole heartbeat run. > - In practice the CLI probe fails transiently (provider hiccup, cold cache, momentary CLI error). When that happens *after* the agent has already done its work, the run dies before its terminal disposition is written, so the platform reopens the issue and re-runs it — a spurious crash/re-run loop that affects every agent on the OpenCode adapter. > - This PR makes the probe **non-fatal when it cannot run**: it warns and proceeds with the configured model, letting the real invocation be authoritative. > - It deliberately **keeps** the genuine guard: when the probe *succeeds* and the configured model is absent from a non-empty list, it still throws (this is what catches misconfigured slugs). > - The benefit is that a best-effort pre-flight check can no longer take down an otherwise-healthy run, while the useful misconfiguration guard is retained. ## Linked Issues or Issue Description No public GitHub issue exists; describing inline (bug). **What happened:** an OpenCode-adapter agent run terminated at the adapter level with `` `opencode models` failed: Unexpected error ``. The failure landed after the agent had produced its work, so the terminal-status update never applied and the run was reopened and re-executed. **Expected:** a transient failure of the `opencode models` availability *probe* should not abort the run — the probe is a best-effort pre-flight guard, not a gate. **Actual:** the probe threw on timeout / non-zero exit / empty output, aborting the run and discarding the completed work + disposition. **Scope:** both the local (`models.ts`) and remote/SSH (`execute.ts`) probe paths; affects any agent on the `opencode_local` adapter. Related PRs (context / prior art): - Refs #5119 — added the remote execution-target model-probe validation this PR softens. - Refs #3291 — closed prior attempt to make the `opencode_local` model probe non-blocking (at agent-create time; different entry point). - Refs #8014 — related open work raising the probe timeout (20s → 60s); complementary, not overlapping. ## What Changed - `models.ts` (`ensureOpenCodeModelConfiguredAndAvailable`): if discovery throws (probe can't run) or returns an empty list, **warn and proceed** with the configured model instead of throwing. The "model present in a non-empty list" check is unchanged and still throws when the configured model is genuinely absent. - `execute.ts` (`ensureRemoteOpenCodeModelConfiguredAndAvailable`): remote probe **timeout / non-zero exit / empty output** now warn and return (proceed) instead of throwing. The remote model-absent guard still throws. - `models.test.ts`: the local "discovery cannot run" case now asserts the probe **proceeds** with the configured model (was: asserts it rejects). - `execute.test.ts`: added remote regression tests — non-zero exit, timeout, and empty output all proceed; a successful probe missing the configured model still rejects. ## Verification ```bash pnpm --filter @paperclipai/adapter-opencode-local typecheck # clean # opencode-local server suite (default 5s per-test timeout is too tight for the # heavy SSH tests on some machines; use a realistic timeout): node node_modules/.pnpm/vitest@*/node_modules/vitest/vitest.mjs run \ packages/adapters/opencode-local/src/server/models.test.ts \ packages/adapters/opencode-local/src/server/execute.test.ts \ packages/adapters/opencode-local/src/server/execute.remote.test.ts \ --testTimeout=45000 ``` Result: typecheck clean; all opencode-local server tests pass, including the new remote fail-open tests and the retained "model unavailable on the remote target" guard test. ## Risks - **Fail-open behavior (intentional).** When the probe can't run, a genuinely misconfigured model is no longer caught at pre-flight — it surfaces at the real invocation instead. This is the accepted tradeoff: the probe is best-effort, and the real invocation is authoritative. The high-value guard (probe succeeds + model absent from a non-empty list) is retained, so the common misconfiguration — a bad `provider/model` slug — is still caught. - No API, schema, or migration changes. Behavior change is confined to the two probe helpers. Low risk overall. ## Model Used Anthropic **Claude Opus 4.8** (`claude-opus-4-8`), used via Claude Code with agentic tool use (repo search, file editing, shell/code execution) and extended reasoning. Used to diagnose the crash, implement the fix, and write the tests; the change was reviewed before submission. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change (`fix/opencode-model-probe-non-fatal`) and contains no internal ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes — N/A (internal adapter behavior; no user-facing docs affected) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green (functional gates: tests/build/e2e/typecheck/security). Review/Greptile gate re-running after this update. - [ ] Greptile is 5/5 with no open P2s — re-triggered after addressing both P2s (remote test coverage + this template-complete description) - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Eric BrookfieldandClaude Fable 5
|
c6727e7b20 |
fix(server): don't implicitly reopen a blocked issue when the same PATCH wires blockers (#10269)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Issues coordinate that work, and first-class blockers
(`blockedByIssueIds`) are how dependent work auto-resumes when its
prerequisites finish
> - A human commenting on a blocked issue implicitly reopens it to
`todo` — a deliberate heuristic so "please continue" comments revive
parked work
> - But that heuristic evaluates the issue's *pre-update* blocker set,
ignoring blockers being wired in by the very same PATCH
> - So the natural repair action for a bare-blocked issue — one PATCH
adding `blockedByIssueIds` plus an explanatory comment — silently flips
the issue to `todo`, contradicting the dependency edit it just made
> - This pull request suppresses the implicit reopen when the request
itself declares a non-empty blocker list
> - The benefit is that structured dependency edits always win over the
conversational-comment heuristic, so blocked issues keep their intended
waiting posture and auto-resume via `issue_blockers_resolved` as
designed
## Linked Issues or Issue Description
No existing issue describes this exact behavior; per the bug-report
template:
- **What happened:** On a `blocked` issue with an empty blocker set, a
board user sent one `PATCH /api/issues/:id` containing
`blockedByIssueIds: ["<unresolved-issue-id>"]` and a `comment`. The
response showed `status: "todo"` — the implicit comment-reopen fired
even though the same request wired an unresolved blocker. A follow-up
`PATCH { status: "blocked" }` was then needed to restore the waiting
posture (and because the blocker array replaces on every update, the two
fields had to be re-sent together).
- **Expected behavior:** A request that explicitly declares dependencies
is stating that the issue is waiting on other work. The implicit reopen
exists for plain conversational comments; it should not override a
structured dependency edit made in the same request.
- **Steps to reproduce:** (1) Create issue A with `status: "blocked"`
and no blockers; (2) as a board user, `PATCH /api/issues/A` with `{
"blockedByIssueIds": ["<id of an open issue>"], "comment": "wiring the
dependency" }`; (3) observe the response/issue status is `todo` instead
of remaining `blocked`.
- **Version/commit:** reproduced on `master` @ `d1b9448b5`.
- **Deployment mode:** `authenticated`, single-host (macOS launchd),
embedded Postgres.
Related (not fixed here): the family of "blocked with empty
`blockedByIssueIds` zombie" reports — Refs #9201, Refs #6523 — this bug
is one way an issue's status and blocker list end up contradicting each
other; and Refs #8062, which proposes a different auto-transition at the
status/blocker boundary.
## What Changed
- `shouldImplicitlyMoveCommentedIssueToTodo`
(server/src/routes/issues.ts) accepts an optional
`requestAddsExplicitBlockers` input and returns `false` when set,
alongside the existing suppression guards, with a comment documenting
the rationale.
- The `PATCH /api/issues/:id` call site passes
`requestAddsExplicitBlockers: Array.isArray(req.body.blockedByIssueIds)
&& req.body.blockedByIssueIds.length > 0`.
- Two route tests in `issue-comment-reopen-routes.test.ts`: a regression
test (comment + non-empty blocker list on a blocked issue must not flip
status) and a boundary test (comment + `blockedByIssueIds: []` still
implicitly reopens, preserving the existing clear-blockers behavior).
Deliberately unchanged: explicit `reopen`/`resume` flags still behave as
before, and the `POST /comments` route is untouched (its body cannot
carry `blockedByIssueIds`).
## Verification
- `cd server && pnpm vitest run
src/__tests__/issue-comment-reopen-routes.test.ts` → 74/74 pass.
- Reverting the `issues.ts` change makes the new regression test fail
with `expected 'todo' to be undefined` — it bites.
- `cd server && pnpm tsc --noEmit` → clean.
## Risks
- Low. The change is a single additional suppression guard on the
*implicit* reopen path, scoped to requests that carry a non-empty
`blockedByIssueIds` array; all other reopen behavior is untouched.
- Edge case considered: a request wiring only already-resolved blockers
plus a comment now stays `blocked` instead of implicitly reopening. This
is the conservative reading of caller intent (an explicit dependency
edit), and an explicit `status`/`reopen` in the same request still wins.
## Model Used
- Anthropic Claude — Fable 5 (`claude-fable-5`), extended thinking
enabled, agentic tool use via Claude Code (CLI). Production repro,
diagnosis, fix, and tests all model-authored under human direction.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes (none
applicable — behavior comment added inline)
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green (pending first CI run on this PR)
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(pending first review pass)
- [x] I will address all Greptile and reviewer comments before
requesting merge
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Jannes Stubbemann
|
6d2eab742f |
fix(server): retry runs that hit a sandbox provider worker restart window instead of failing setup (#10212)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agent runs execute in sandbox environments acquired through provider
plugins (e.g. the Kubernetes sandbox provider)
> - Lease acquisition happens during run setup, before the adapter
executes
> - When a provider plugin's worker is momentarily unavailable (a server
or plugin restart window), lease acquisition throws "Sandbox provider
... is installed via plugin ..., but its worker is not running."
> - The heartbeat setup path records that as a terminal `setup_failed`:
no retry classifier matches the message, so the run dies instantly even
though the worker returns seconds later
> - This PR classifies that transient condition as retryable
infrastructure so the run is retried instead of being lost to a restart
blip
> - The benefit is that routine restarts no longer produce spurious
instant run failures
## Linked Issues or Issue Description
No public GitHub issue exists; describing inline following the bug
report template.
**What happened**
During a brief sandbox-provider-worker restart window, several runs
failed instantly with `setup_failed` ("... but its worker is not
running."), while runs on the same agent moments earlier and later
succeeded.
**Expected behavior**
A transient, self-healing worker-unavailable condition should schedule a
bounded retry, not terminally fail the run.
**Steps to reproduce**
Trigger a run while the sandbox provider plugin worker is momentarily
unavailable (a server or plugin restart). Lease acquisition throws the
worker-not-running error and the run is finalized as `setup_failed` with
no retry. The recovery test added here reproduces the classification
path.
**Deployment mode**
Cloud multi-tenant execution (Kubernetes sandbox provider plugin).
## What Changed
- Added a dedicated, readable predicate that recognizes the transient
sandbox-provider-worker-unavailable lease failure and treats it as
retryable infrastructure, so the heartbeat schedules a bounded
continuation retry instead of finalizing terminally
- The predicate is anchored to the full lease-failure phrasing (`is
installed via plugin ... but its worker is not running`) so it cannot
match the permanent "provider not installed" message emitted by config
validation
- Added tests proving the readiness poll already waits the full deadline
while the worker handle is absent or `starting` (registered-late
coverage); no poll behavior change was needed
## Verification
- `cd server && npx vitest run
src/__tests__/environment-runtime.test.ts` — poll exhaustion +
registered-late cases
- `npx vitest run src/__tests__/heartbeat-process-recovery.test.ts` —
worker-unavailable message schedules a retry; a non-matching permanent
provider failure still escalates terminally (negative case)
## Risks
Low risk. The retry is bounded by the existing
infrastructure-continuation attempt cap (max 3), the message match is
narrow enough to exclude the permanent provider-not-installed failure
(covered by a negative test), and no readiness-poll or lease-acquisition
behavior changed.
## Model Used
Claude (Anthropic) via Claude Code. Implementation and tests authored by
a Claude Sonnet-class model (`claude-sonnet-5`) dispatched as isolated
per-task implementer agents under a multi-agent orchestration workflow;
root-cause investigation, planning, and two-stage adversarial code
review performed by additional Claude agents. Extended thinking and tool
use enabled throughout.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
|
||
|
Jannes StubbemannandClaude Opus 5
|
a0bdf388af |
fix(agents): refuse to hire onto an adapter this instance cannot run (#10256)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Hiring an agent means choosing a harness (adapter) for it, and an
instance can declare which harnesses it actually runs through
`PAPERCLIP_ADAPTERS`, which `reconcileAdapterAvailability` turns into a
disabled set at boot
> - The hire and create routes validate the adapter type with
`assertKnownAdapterType`, which only asks whether the adapter is
REGISTERED — a disabled adapter passes
> - So an agent can be created on a harness the instance cannot run, and
the failure only appears later, per run, at lease time: `Adapter "..."
is not in the configured adapter registry`
> - By then the error is in a run log, minutes after the choice, with
nothing tying it back to the harness the user picked; the agent also
keeps accepting work it can never do
> - This pull request validates the hire and create paths against the
ENABLED set and refuses with a message that names the adapters that are
available
> - The benefit is that an impossible choice fails at the moment it is
made, in the words of the choice itself, instead of as a run failure the
user cannot act on
## Linked Issues or Issue Description
No existing issue; describing it here per the bug report template.
**What happened**
On an instance with a curated registry, a company's Chief of Staff was
hired on `cursor_cloud`, which that instance had disabled. The API
accepted the hire. Its first assignment run then failed:
```
Failed to acquire lease for environment "Kubernetes Sandbox" (sandbox): Adapter "cursor_cloud" is not in the configured adapter registry
```
and its automation run sat in `queued` for hours afterwards. Nothing in
the hire response, the agent detail view, or the agent's status
explained that this harness could never run.
**Expected behavior**
hiring on an adapter the instance has disabled is refused at hire time,
with a message naming the adapters that can be chosen.
**Steps to reproduce**
1. Start the server with a registry that omits an otherwise-registered
adapter, e.g. `PAPERCLIP_ADAPTERS` listing `claude_local` but not
`cursor_cloud`.
2. `POST /api/companies/:companyId/agents` with
`{"name":"CoS","adapterType":"cursor_cloud"}`.
3. The agent is created (201). Every run it attempts fails at lease time
with the message above.
**Paperclip version or commit**
master (`4c55f0d8d`).
## What Changed
- `server/src/routes/agents.ts`: adds `assertSelectableAdapterType`,
which extends `assertKnownAdapterType` with an enabled-set check and
throws `422 Adapter "<type>" is not available on this instance.
Available adapters: <list>`. The hire (`POST .../agent-hires`) and
create (`POST .../agents`) paths now use it.
- Routes that operate on an EXISTING agent keep
`assertKnownAdapterType`, so an agent already running on a
since-disabled adapter is unaffected — the same rule
`listEnabledServerAdapters` already documents ("hidden from selection,
still functional for agents that already use them").
- `server/src/__tests__/agent-adapter-validation-routes.test.ts`: mocks
the adapter-plugin store's disabled set (so the test never writes to a
real `~/.paperclip/adapter-settings.json`), and covers
refuse-when-disabled (including that the message names the alternatives
and that no agent is created) plus create-still-works-when-enabled.
## Verification
```
pnpm vitest run server/src/__tests__/agent-adapter-validation-routes.test.ts
```
13 tests pass, including the two new cases and the existing
unknown-adapter-type test.
Manual: disable an adapter (`PATCH /api/adapters/:type {"disabled":
true}` as an instance admin, or omit it from `PAPERCLIP_ADAPTERS` and
restart), then POST an agent with that `adapterType` — 422 naming the
available adapters, and no agent row is created.
## Risks
Low, and scoped to new selections:
- Automation that creates agents on a disabled adapter now gets a 422
where it previously got a 201 followed by runs that always failed. That
is the intended behavior change, and the message names the valid
choices.
- Existing agents, and every route that acts on an existing agent, are
untouched.
- The enabled set comes from the same store `GET /api/adapters` already
reports, so the API and the picker cannot disagree.
## Model Used
Claude Opus 5 (Anthropic), model id `claude-opus-5`, 1M context window,
extended thinking, with tool use and code execution 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
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change
(`upstream/adapter-selection-guard`) and contains no internal ticket id
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes (the
new helper documents the selection-vs-existing-agent rule)
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
Related: #10254 makes the adapter inventory readable during onboarding,
which is what lets the picker hide these adapters in the first place.
This PR is the server-side backstop for the same failure.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Constantineandcucurigoo
|
68c699687e |
fix(ui): keep selected run lifecycle and logs current (#10169)
## Thinking Path > - Paperclip's agent-run detail view combines live lifecycle events, detail queries, and persisted shell logs. > - Queued runs do not have a log reference yet, so polling the log endpoint returns expected-but-noisy 404s. > - Simply disabling queued log polling can strand the selected detail cache at `queued` or leave terminal lifecycle fields stale. > - The detail cache therefore needs both live lifecycle patches and an HTTP fallback while the run is active. > - Both the selected-run query and the shared transcript hook must avoid reading persisted logs before execution starts. > - This pull request disables queued log polling in both paths, patches lifecycle fields, invalidates authoritative detail data, and polls run state only while active. > - The benefit is quiet queued runs, reliable queued-to-running transitions, and complete final log handoff. ## Linked Issues or Issue Description No exact duplicate found. I searched open PRs for `queued run`, `log polling`, and `run lifecycle`; related results addressed stale queued execution or dashboards rather than selected-run log/detail synchronization. **What happened?** Opening or rendering a queued run caused repeated `GET /api/heartbeat-runs/:id/log` 404s from both the selected detail and shared transcript hydration paths. A queued-only detail guard could also leave the selected run stuck at `queued` when live delivery was missed, or stop before fetching final `logRef` and terminal fields. **Expected behavior** Queued runs should not request unavailable logs. The selected detail should transition through running to terminal via live events or bounded HTTP fallback, refresh authoritative detail data, fetch final logs, and then stop polling. **Steps to reproduce** 1. Open the detail page for a run waiting behind the agent concurrency limit. 2. Observe repeated log-endpoint 404s while status is queued. 3. Let the run start with WebSocket delivery unavailable or delayed. 4. Observe stale detail state without a run-detail fallback. **Environment** - Paperclip base: `14f20be92b86a49ff2c35495e5b0fa4d719998ef` - Deployment: self-hosted, built from source - Adapter scope: visible with Hermes-backed agents but not adapter-specific - [x] I searched open PRs for queued-run log polling and selected-run lifecycle synchronization; no exact duplicate was found. ## What Changed - Poll shell logs only while a run is `running`, never while `queued`. - Defer shared persisted-transcript hydration and its live WebSocket until a queued run becomes `running`; terminal runs still receive one persisted-log hydration attempt. - Patch selected run status, invocation metadata, errors, and start/finish times from lifecycle events. - Invalidate selected run detail on lifecycle events to hydrate authoritative `logRef`, result, usage, and excerpts. - Poll run detail every 5 seconds while queued and every 15 seconds while running, then stop at terminal status. - Add regression coverage for queued/running/terminal polling and lifecycle cache handoff. ## Verification - `pnpm exec vitest run ui/src/context/LiveUpdatesProvider.test.ts ui/src/pages/AgentDetail.progress.test.ts` — 42 passed. - `pnpm exec vitest run ui/src/components/transcript/useLiveRunTranscripts.test.tsx` — 11 passed. - `pnpm --filter @paperclipai/ui typecheck` - `pnpm --filter @paperclipai/ui build` - Exact final diff independently reviewed: **APPROVE**, no blocking findings. - UI-only local cutover completed with the Paperclip server PID unchanged. ## Risks - The selected run performs one lightweight detail GET every 5 seconds while queued and every 15 seconds while running if it remains open. Polling stops at terminal status. - Lifecycle events still patch immediately; polling is only the fallback and authoritative hydration path. - No API, schema, or migration changes. > This is a bug fix, not roadmap feature work. ## Model Used OpenAI Codex `gpt-5.6-sol`, with tool use, code execution, repository inspection, and independent read-only review agents. ## 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/described the search above - [x] I have described the issue in-PR following the bug template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have considered documentation; no user-facing documentation change is required - [x] I have considered and documented risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: cucurigoo <cucurigoo@users.noreply.github.com> |
||
|
Constantineandcucurigoo
|
106955bdfd |
fix(ui): gate summary built-in requests by feature flag (#10170)
## Thinking Path > - Paperclip's web UI conditionally exposes experimental summary and built-in-agent capabilities. > - Summary cards depend on the built-in Summarizer agent, but the two capabilities have independent feature flags. > - `SummarySlotCard` and the reusable `BuiltInAgentGate` enabled built-in-agent lookups without requiring `enableBuiltInAgents`. > - When built-in agents were disabled, those surfaces called a server route that was intentionally unavailable and generated avoidable 404s. > - The client query should obey both server-side feature gates. > - This pull request adds the missing gate and a cross-flag regression. > - The benefit is consistent feature-flag behavior and no request loop against a disabled endpoint. ## Linked Issues or Issue Description No exact duplicate found. I searched open PRs for `SummarySlotCard`, `BuiltInAgentGate`, `enableBuiltInAgents summaries`, and `built-in agents 404`. Related PR #10116 gates `SidebarAgents`; this PR deliberately excludes that file and covers the remaining summary/gate callers. **What happened?** `SummarySlotCard` called `builtInAgentsApi.list` whenever summaries were enabled, and `BuiltInAgentGate` called it whenever a company was selected. The server rejects that route when `enableBuiltInAgents` is false, so the disabled configuration produced repeated 404 requests. **Expected behavior** Built-in-agent queries should run only when built-in agents are enabled; the summary-specific query also requires summaries to be enabled. **Steps to reproduce** 1. Enable summaries. 2. Disable built-in agents. 3. Render a page containing `SummarySlotCard` or `BuiltInAgentGate`. 4. Observe a request to the disabled built-in-agents route. **Environment** - Paperclip web UI - Cross-flag configuration: summaries enabled, built-in agents disabled - [x] I searched open PRs for the affected component, feature flags, and 404 behavior; no exact duplicate was found. ## What Changed - Require both `enableSummaries` and `enableBuiltInAgents` in `SummarySlotCard`. - Make `BuiltInAgentGate` resolve experimental settings before enabling its built-in-agent query and fail open when the feature is disabled. - Add cross-flag regressions for both callers. - Leave `SidebarAgents` to related PR #10116 rather than duplicating it. ## Verification - `pnpm exec vitest run ui/src/components/SummarySlotCard.test.tsx ui/src/components/BuiltInAgentGate.test.tsx` — passed as part of a 41-test built-in UI group. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm --filter @paperclipai/ui build` — passed in combined deployment staging. - UI-only local cutover completed with the Paperclip server PID unchanged. - The complete UI fix was staged after a summary-only cutover exposed the remaining reusable-gate caller. ## Risks - Low risk: this changes only whether one query is enabled under a feature-flag combination where the server route is unavailable. - No API, schema, migration, authentication, or persistence changes. - Rollback is a single commit revert. > This is a bug fix, not roadmap feature work. ## Model Used OpenAI Codex `gpt-5.6-sol`, with tool use, code execution, repository inspection, and read-only review-agent evidence. ## 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 described the search above - [x] I have described the issue in-PR following the bug template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have considered documentation; no user-facing documentation change is required - [x] I have considered and documented risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: cucurigoo <cucurigoo@users.noreply.github.com> |
||
|
Eric BrookfieldandClaude Opus 4.8
|
20482a4cb6 |
fix(server): gate heartbeat-fallback comment to never publish raw transcript (#10143)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agents run on a heartbeat; when an issue-scoped run ends, the server
records the outcome on the issue's board thread.
> - Normally the agent posts its own summary comment via `POST
/comments`. When it doesn't, the server has a fallback that
auto-publishes a run summary so the board isn't left silent.
> - That fallback (`buildHeartbeatRunIssueComment` in
`server/src/services/heartbeat-run-summary.ts`) returns
`resultJson.summary` **verbatim**, with no length cap or shape check.
> - For runs that never produce a final `result`, `summary` is
concatenated **inter-tool narration** ("Let me check…", "I'll fetch…",
joined by the claude-local adapter's parser). The fallback then dumps
that raw transcript onto the public board thread.
> - In practice this produces long, confusing transcript comments that
mislead reviewers and other agents about what actually happened.
> - This PR gates the fallback so it publishes a clean summary or a
short stub, never raw transcript.
> - The benefit is that the board thread stays trustworthy: a missing
agent summary degrades to a one-line "no summary this run" note instead
of leaking internal narration.
## Linked Issues or Issue Description
No public GitHub issue exists for this; describing it here as a bug
report.
**What happened:** When an issue-scoped heartbeat run finishes without
the agent posting its own comment, the server's fallback publishes
`resultJson.summary` verbatim as the board comment. When the run
produced no final result, that value is concatenated inter-tool
narration, so raw transcript is posted to the issue thread.
**Expected behavior:** The fallback should post a concise summary when
one is available, and otherwise a short stub — never multi-hundred-line
raw narration.
**Steps to reproduce:**
1. Run an issue-scoped agent turn that ends without calling `POST
/comments` and without emitting a final `result` (only inter-tool
narration).
2. Observe the auto-published board comment: it is the full narration
transcript.
**Deployment mode:** self-hosted server
(`server/src/services/heartbeat.ts` fallback path).
**Prior attempt:** an earlier PR for this change was auto-closed when
its head branch was renamed to strip an internal ticket id from the
branch name; this PR supersedes it.
**Related PR:** #7505 (`fix(heartbeat): skip auto-mirror run-summary
comment on cross-owner wakes`) touches the same fallback area but
addresses a different case (cross-owner wakes); this PR is
complementary, gating the *content* of the fallback rather than *when*
it fires.
## What Changed
- `server/src/services/heartbeat-run-summary.ts`:
`buildHeartbeatRunIssueComment` now gates the fallback text. After
resolving `summary` → `result` → `message`, if the text opens with a
narration phrase (`let me`, `i'll`, `i need to`, `i can see`, `looking
at`, `fetching`, `checking`, `first,`) **or** exceeds
`MAX_FALLBACK_COMMENT_CHARS` (1200), it returns a fixed stub: *"Run
completed. Agent did not post a summary comment this run (transcript
withheld — see run log)."* Otherwise it returns the text unchanged.
- `server/src/__tests__/heartbeat-run-summary.test.ts`: added cases for
each narration opener, the length cap, the exact 1200-char boundary
(posts), and clean-summary passthrough.
Runs where the agent posts via the API are unaffected — the fallback
only fires when no agent comment is found for the run, and that call
site is unchanged.
## Verification
- `pnpm --filter @paperclip/server test heartbeat-run-summary` — 13/13
pass (new + existing cases).
- Manual reasoning: the gate is a pure function of the resolved text;
API-posted runs never reach it.
- **CI note:** at the time of opening, `pnpm install --frozen-lockfile`
fails on this branch's base commit with
`ERR_PNPM_LOCKFILE_CONFIG_MISMATCH` (patchedDependencies drift). This
reproduces on every PR based on the current `master` tip (e.g. #10137)
and is unrelated to this two-file change; PRs cut from the prior master
(e.g. #10135) install cleanly. This should clear once the `Refresh
Lockfile` job lands a corrected lockfile on `master` and this branch is
rebased. Happy to rebase or fold in the lockfile fix if a maintainer
prefers.
## Risks
Low risk. The change is confined to one pure function and its tests,
touches no schema or migration, and only alters the *fallback* comment
path (never the normal API-posted path). Worst case is a legitimate
clean summary that happens to open with a gated phrase gets replaced by
the stub — the run log still holds the full detail.
## Model Used
Claude Opus 4.8 (`claude-opus-4-8`), 1M-token context window, extended
thinking, with tool use.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change
(`fix/gate-heartbeat-fallback-comment`) and contains no internal
Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes (no
user-facing docs affected)
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green (blocked on a master-side
lockfile drift, see CI note)
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(pending re-review on this PR)
- [x] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Constantineandcucurigoo
|
b42efe2063 |
fix(ui): gate built-in agent sidebar query (#10116)
## Thinking Path > - Paperclip is the open-source control plane people use to manage AI agents and their work. > - The sidebar renders company agents and, when enabled, their built-in lifecycle state. > - The built-in-agents API is intentionally unavailable when the experimental feature is disabled. > - `SidebarAgents` nevertheless queried that endpoint whenever a company was selected, producing routine 404s in normal feature-off installations. > - The query must be gated by the shared instance setting, including its unresolved state, without exposing stale cached lifecycle badges. > - This pull request adds that gate and focused disabled, unresolved, and enabled regressions. > - The benefit is a quiet sidebar network path with unchanged behavior for installations that enable built-in agents. ## Linked Issues or Issue Description ### Pre-submission checklist - [x] I searched existing open and closed issues/PRs and found no exact duplicate. Related but not duplicate: #4149. - [x] I reproduced this on current `master`. - [x] I confirmed the error originates in Paperclip's sidebar query gating, not an adapter, provider, or local configuration. ### What happened? With `enableBuiltInAgents: false`, mounting `SidebarAgents` for a selected company still called `GET /api/companies/:companyId/built-in-agents`. The server correctly returned 404 because the experimental feature was disabled. ### Expected behavior The sidebar must not call the built-in-agent endpoint until the shared experimental setting resolves to exactly `true`. Cached built-in lifecycle state must also remain hidden while disabled. ### Steps to reproduce 1. Set `enableBuiltInAgents` to `false`. 2. Open any company so the sidebar agent list mounts. 3. Observe a request to `/api/companies/:companyId/built-in-agents` and a 404 response. ### Environment - Paperclip commit: `f2f168f6a10a24c924516808f414baba52b1c080` - Deployment mode: self-hosted server - Installation method: built from source - Adapter: not adapter-specific (core UI bug) - Database mode: not database-related - Access context: board (human operator) - Node.js: `v22.22.3` - Operating system: Linux - Relevant config: `{"enableBuiltInAgents": false}` - Relevant output: redacted HTTP 404 from `/api/companies/:companyId/built-in-agents` - Privacy: all instance-local identifiers, paths, and output were omitted or redacted. ## What Changed - Read instance experimental settings in `SidebarAgents` through the shared React Query key. - Enable the built-in-agent list query only when `enableBuiltInAgents` is explicitly `true`. - Ignore cached built-in lifecycle data while the feature is disabled. - Add regressions for disabled, unresolved, and enabled settings states. ## Verification - RED before implementation: the disabled-feature regression failed because `builtInAgentsApi.list` was called once. - `pnpm --filter @paperclipai/ui exec vitest run src/components/SidebarAgents.test.tsx` — 24 tests passed. - `pnpm --filter @paperclipai/ui exec tsc -p tsconfig.json --pretty false` — passed. - `git diff --check` — passed. - The existing test file emits pre-existing React `act(...)` warnings while passing. ## Risks - Low risk: request gating only; no API, schema, migration, or visible UI contract changes. - A stale experimental-settings cache could delay enabling the query until the normal settings invalidation/refetch path runs; this is the same shared query key already used elsewhere. - Cached built-in statuses are deliberately hidden whenever the setting is not literal `true`. > 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 `gpt-5.6-sol` was used with reasoning, repository/file tools, shell command execution, and delegated read-only code review. The runtime did not expose a context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (no documentation change needed; this enforces the existing feature flag) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: cucurigoo <cucurigoo@users.noreply.github.com> |
||
|
Constantineandcucurigoo
|
2f1c0e011e |
fix(hermes): surface silent nonzero exit failures (#10107)
## Thinking Path - Followed a silent nonzero Hermes exit from child-process result parsing through heartbeat run, runtime, task-session, and agent finalization. - Found two gaps: the adapter could return `errorMessage: null` for a numeric nonzero exit, and heartbeat later reused the nullable adapter field instead of its normalized fallback. - Kept timeout, signal-cancellation, and specific parsed diagnostics authoritative. ## Linked Issue(s) / Bug Report Related to #9751 (stderr classification) and #9519 (exit-zero finalization), but this is a separate failure mode. Reproduction: run Hermes with a child result equivalent to `exitCode: 1`, `timedOut: false`, and no parsed diagnostic. The heartbeat row derives `Adapter failed`, while runtime/task-session/agent finalization can persist null diagnostics. ## What Changed - Give silent numeric nonzero Hermes exits a stable fallback such as `Hermes exited with code 1`. - Preserve specific parsed errors and timeout/signal semantics. - Reuse the normalized persisted run error for recovered runtime state, task-session `lastError`, and agent `errorReason`. - Add adapter-level and embedded-Postgres regressions. ## Verification - Hermes adapter `execute.onspawn.test.ts` — 7 passed. - Focused heartbeat normalized-error regression — 1 passed (91 skipped). - `pnpm --filter @paperclipai/hermes-paperclip-adapter typecheck` — passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - `git diff --check origin/master...HEAD` — passed. Independent review also ran the full recovery file: the changed regression passed; one unrelated pre-existing timing-sensitive test timed out. ## Risks / Rollout Notes Low risk. Fallback text is used only when a numeric nonzero exit has no better diagnostic. Existing timeout, signal, and parsed-error precedence remains unchanged. ## Model Used OpenAI Codex `gpt-5.6-sol` with repository inspection, test execution, and independent read-only review. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (not applicable: internal diagnostics only) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: cucurigoo <cucurigoo@users.noreply.github.com>canary/v2026.812.0-canary.15 |
||
|
Jannes StubbemannandClaude Fable 5
|
8a5c0615f9 |
fix(adapter-utils): forward sandbox callback bridge traffic to the local listen origin (#10017)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents can execute in remote sandboxes, where a callback bridge relays in-sandbox Paperclip API calls back to the host server process > - The bridge worker resolves its forward target from PAPERCLIP_RUNTIME_API_URL / PAPERCLIP_API_URL, which now prefer a configured public base URL and therefore mean "the origin browsers and external agents use" > - The bridge worker runs inside the same process that serves the API, so forwarding through the public origin routes an in-process loopback hop through the network edge > - On a deployment whose public origin sits behind a session-gated edge proxy, every forwarded agent API call is rejected at the edge, so agents in sandboxes cannot read their identity, comment, or hire > - This pull request resolves the bridge forward target from the explicit hostApiUrl override or the local listen host and port only, never the public URL exports > - The benefit is that sandbox agent API calls keep working regardless of how the public base URL is configured or gated ## Linked Issues or Issue Description No existing issue. Describing in-PR following the bug report template: **What happened?** On a cloud deployment with a session-gated public edge, setting a public base URL (PAPERCLIP_PUBLIC_URL) caused every in-sandbox agent API call through the sandbox callback bridge to fail with `403 text/plain "Access denied"` from the edge proxy. With PAPERCLIP_BRIDGE_DEBUG enabled, the bridge logs show the forward target is the public origin, and every proxied request (for example `GET /api/agents/me`) returns the edge proxy's 403 instead of reaching the API. **Expected behavior** The bridge worker runs in the same server process that serves the API, so forwarded calls should target the local listen origin and succeed regardless of how the public origin is configured or gated. **Steps to reproduce** 1. Run the server with a public base URL configured, fronted by a proxy that requires a browser session on API routes. 2. Start a sandbox-executed agent run (any adapter using the sandbox callback bridge). 3. Observe every in-sandbox call to the Paperclip API fail with the proxy's 403; with PAPERCLIP_BRIDGE_DEBUG the forward URL is the public origin. **Paperclip version or commit** Current `master`. **Deployment mode** Self-hosted server behind a reverse proxy. **Agent adapter(s) involved** All sandbox-executed adapters (the bridge is adapter-agnostic). ## What Changed - `packages/adapter-utils/src/execution-target.ts`: `startAdapterExecutionTargetPaperclipBridge` now resolves its forward target as `input.hostApiUrl?.trim() || resolveDefaultPaperclipApiUrl()`. It no longer consults `PAPERCLIP_RUNTIME_API_URL` / `PAPERCLIP_API_URL`, which now describe the public origin for browsers and external agents, exactly the wrong target for an in-process loopback hop. `resolveDefaultPaperclipApiUrl()` builds `http://<PAPERCLIP_LISTEN_HOST>:<PAPERCLIP_LISTEN_PORT>` (exported by server boot before any run executes) and maps wildcard listen hosts to the loopback address of the same family (`0.0.0.0` to `127.0.0.1`, `::` to `[::1]`), so the forward target always matches the address family the server is bound to. `input.hostApiUrl` remains the explicit override seam. A comment documents the reasoning. - `packages/adapter-utils/src/execution-target-sandbox.test.ts`: two new tests. One sets both public URL env vars to an unreachable public https origin and asserts the bridge forwards to the local listen origin (fails before this fix with a 502 because the worker targets the public origin). One asserts an explicit `hostApiUrl` input still overrides everything. - The acpx-engine bridge start (`packages/adapter-utils/src/acpx-engine/execute.ts`) passes no `hostApiUrl` and goes through the same resolution site, so it is covered by the same fix. The sandbox-facing env builder in `server-utils.ts` is intentionally untouched; the bridge env overrides `PAPERCLIP_API_URL` inside the sandbox separately. ## Verification - `npx vitest run packages/adapter-utils/src/execution-target-sandbox.test.ts` (28 tests pass; the new local-origin test fails without the fix) - `pnpm --filter @paperclipai/adapter-utils typecheck` (clean) - Full adapter-utils suite run; the only failures are pre-existing environment-dependent tests (bubblewrap and shallow-clone tests on macOS) identical on a clean `master` checkout ## Risks - Low risk. Deployments where the bridge previously worked did so precisely because the forward target already resolved to the local origin (no public URL configured, so the chain fell through to the same `resolveDefaultPaperclipApiUrl()` result). The only behavioral shift is for deployments with a public URL configured, where forwarding through the edge was either wasteful (an unnecessary network round trip) or broken (session-gated edge). The explicit `hostApiUrl` override seam is preserved for callers that need a nonlocal target. ## Model Used - Claude Fable 5 (claude-fable-5), extended thinking, 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 - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Daniel SauerandDaniel Sauer
|
91669741d2 |
fix(server): close tool-access cross-tenant ID oracles (#9589)
## Thinking Path > - Paperclip's company-scoped HTTP routes must reject inaccessible resources before returning resource-specific authorization results. > - The shared `getAccessibleResource` helper established that invariant, but direct tool-access routes still fetched globally unique IDs first and then returned 403 from later authorization checks. > - A signed-in user could therefore distinguish a valid foreign-company resource ID from an unknown ID. > - This change applies the existing tenant-aware lookup gate consistently across direct tool-resource routes and rejects inaccessible OAuth state before callback-specific authorization. ## Linked Issues or Issue Description - No standalone issue exists. This is a security-hardening follow-up to #3967. - **Observed:** a member of company A can submit a known application, connection, profile, profile-entry, or OAuth-state ID belonging to company B and receive a different response than for a random missing ID. - **Expected:** missing and inaccessible foreign resources are indistinguishable at the HTTP boundary. Signed-in instance administrators still require company membership for company-scoped access. - **Reproduction:** create resources in company B, authenticate as an owner of company A without B membership, and call the direct `/api/tool-*` routes using B's IDs. Before this change, affected calls returned 403 while unknown IDs returned 404. ## What Changed - Wrapped direct application, connection, profile, and profile-entry lookups in `server/src/routes/tool-access.ts` with the shared `getAccessibleResource` 404 gate. - Added tenant membership validation to OAuth callback-state lookup before session/role checks, returning the same invalid-state response as an unknown state. - Expanded route regressions across connection/profile endpoint families, including grants, usage, installs, gateway-backed test calls, OAuth, mutations, catalog/activity reads, profile entries, and instance-admin-without-membership access. - Updated application update/delete expectations from cross-tenant 403 to non-enumerating 404 responses. ## Verification After rebasing onto current `master`: - `pnpm exec vitest run src/__tests__/tool-access-service.test.ts` from `server/` — 113 passed. - `pnpm --filter @paperclipai/server typecheck` — previously passed on the same implementation; affected upstream paths were unchanged before this mechanical rebase. ## Risks - Low implementation risk: no schema, migration, or successful same-company response changes. - Intentional behavior change: inaccessible foreign tool-resource IDs now return 404 instead of 403; inaccessible OAuth states return the same 400 body as missing/expired states. - The gate reuses `getAccessibleResource` / `hasCompanyAccess` semantics established by #3967. > 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, exact model ID `openai-codex/gpt-5.6-sol`; repository, shell, test, TypeScript language-server, and GitHub CLI tool access 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 - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either linked an existing issue or described the issue in-PR - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket ID - [x] I have run focused tests locally on the final rebased head and they pass - [x] I have added or updated tests where applicable - [x] Documentation update — N/A: internal authorization correction only - [x] I have considered and documented risks above - [ ] All Paperclip CI gates are green on the new rebased head - [x] Greptile's prior review was 5/5 with no open findings - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Daniel Sauer <sauerdaniel@users.noreply.github.com> |
||
|
Jannes StubbemannandClaude Fable 5
|
1f7959bc69 |
fix(codex-local): skip benign stderr warnings when deriving the fallback run error (#10003)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents execute through adapters; the codex_local adapter runs the Codex CLI and reports each run's outcome, including an error message when the CLI exits nonzero > - When no error can be parsed from the CLI's JSONL output, `toResult` in `packages/adapters/codex-local/src/server/execute.ts` falls back to the first non-empty stderr line as the run error > - The adapter itself passes the approvals-bypass flag, so the CLI's first stderr line is always the benign startup warning "YOLO mode is enabled. All tool calls will be automatically approved." > - Failed runs therefore record that warning as their error, hiding the real cause (for example an OpenAI API error further down in stderr) and making failures hard to diagnose from the run record > - This pull request derives the fallback error from the first meaningful stderr line, skipping a conservative set of known benign lines, and keeps the existing behavior when every line is benign > - The benefit is that failed Codex runs surface the actual failure reason instead of a harmless startup warning, without ever producing an emptier message than before ## Linked Issues or Issue Description No public issue exists for the codex_local case. The same bug class was fixed for gemini-local in Refs #5099 and Refs #3476; this PR applies the equivalent fix to codex_local. **What happened?** On a multi-tenant cloud deployment of Paperclip, several codex_local runs failed and their run records showed `error_code=adapter_failed` with the error text "YOLO mode is enabled. All tool calls will be automatically approved." That is a benign Codex CLI startup warning, printed on every run because the adapter passes the approvals-bypass flag itself. The real failure (an OpenAI API error printed later in stderr) was never surfaced. **Expected behavior** When the Codex CLI exits nonzero and no error was parsed from its JSONL output, the run error should be the first stderr line that actually explains the failure, not a startup warning the adapter itself provoked. **Steps to reproduce** 1. Configure a codex_local agent and make the underlying Codex CLI invocation fail after startup (for example, configure a model id the active credentials cannot use). 2. Run the agent so the CLI exits nonzero with no parsed JSONL error. 3. Inspect the run's error message: it shows the YOLO approvals warning (the first stderr line) instead of the real error printed further down in stderr. ## What Changed - Added `firstMeaningfulStderrLine` next to `firstNonEmptyLine` in `packages/adapters/codex-local/src/server/execute.ts`, with a conservative benign-line predicate covering the YOLO approvals warning and `[paperclip] ...` diagnostic lines the adapter injected (for example ACP fallback notes). - Used it only in the `toResult` fallback error derivation. If every stderr line is benign, the existing chain still applies (first non-empty line, then `Codex exited with code N`), so the message never gets emptier than today. Logging is unchanged. - Added `packages/adapters/codex-local/src/server/execute.stderr-error.test.ts`: four end-to-end cases through `execute()` with a mocked CLI process, plus unit coverage for the new helper. Tests were written first and confirmed failing before the fix. ## Verification - `pnpm exec vitest run packages/adapters/codex-local/src/server/execute.stderr-error.test.ts` (7 tests pass; 5 failed before the fix as expected) - `pnpm exec vitest run packages/adapters/codex-local` (21 files, 188 tests pass) - `pnpm run typecheck` in `packages/adapters/codex-local` (clean) ## Risks Low risk. Only the derived fallback `errorMessage` changes, and only when a benign line would otherwise have been picked; parsed JSONL errors, logging, retry/quota/auth classification inputs, and the empty-stderr exit-code fallback are untouched. The benign-line list is deliberately conservative (exact prefixes) so real errors are never skipped. ## Model Used Claude Fable 5 (claude-fable-5), extended thinking, 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 - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Jonathan Reyes
|
676e20a894 |
fix(routines): reject HMAC webhook replays (#9994)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Routines allow external systems to start recurring work through authenticated public webhooks > - Timestamped HMAC authentication currently verifies authenticity and age but does not remember an accepted delivery > - An exact signed request can therefore be reused within its replay window, including through simultaneous duplicate delivery > - Replay rejection must be atomic with run creation so concurrent copies cannot both succeed > - This pull request derives a non-secret replay identity from each valid timestamped HMAC delivery and claims it under the existing routine transaction lock > - The benefit is at-most-once acceptance of an exact HMAC delivery without changing ordinary caller-supplied idempotency semantics ## Linked Issues or Issue Description Fixes: #9993 ## What Changed - Derive a stable, non-secret idempotency key after a timestamped HMAC signature has been validated. - Reject a previously claimed HMAC delivery with a conflict while preserving coalescing for existing non-HMAC idempotency keys. - Apply the same atomic replay claim when automatic worktree execution is suppressed. - Add regression coverage for sequential, concurrent, and suppressed-run replays. ## Verification - `pnpm exec vitest run server/src/__tests__/routines-service.test.ts` — 59 tests passed. - `pnpm typecheck` — all workspace packages passed. - The sequential test was observed failing on unmodified `master`: the second identical request resolved and a second run was created. - The concurrent regression test verifies exactly one request succeeds and only one routine run exists. ## Risks - Low migration risk: no schema change is required; the existing nullable routine-run idempotency field is reused. - The routine row lock serializes replay claims, adding a small amount of contention only while a routine run is being created. - Replay rejection applies only to `hmac_sha256`, which carries the timestamp needed for a bounded replay policy. Existing `github_hmac`, bearer, and unauthenticated trigger semantics are unchanged. > 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 (GPT-5 family) with reasoning, repository inspection, shell execution, and test tooling. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] Documentation does not require an update because this restores the documented replay-window security behavior without changing configuration or APIs - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Christian Lappin
|
fc5c6ffed2 |
fix(server): return 404 instead of 500 for non-UUID company refs (#9959)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The board REST API is how operators and integrations read company
state; `GET /api/companies/:companyId` is one of its most basic reads
> - The route passes the raw path param into `companyService.getById`,
which queries the uuid-typed `companies.id` column directly
> - Any non-UUID ref — a slug, a typo, a stale bookmark — makes Postgres
throw `invalid input syntax for type uuid`, which surfaces as an HTTP
500 with a stack trace in the server log instead of a clean client error
> - A 500 for malformed client input is miscategorized: it pages
operators, pollutes error budgets, and hides the actual problem ("that
ref doesn't exist") from the caller
> - This pull request guards `getById` with a UUID check so non-UUID
refs resolve to `null` and the route returns its existing 404 path
> - The benefit is correct HTTP semantics for bad input, quieter logs,
and one less misleading 500 for self-hosters to chase
## Linked Issues or Issue Description
Fixes #9962 — `GET /api/companies/:companyId` returns 500 (`invalid
input syntax for type uuid`) for non-UUID refs instead of 404. Full
repro and log excerpt in the issue.
## What Changed
- `server/src/services/companies.ts`: `getById` returns `null` early for
non-UUID refs instead of passing them to the uuid-typed query.
- `server/src/__tests__/companies-service.test.ts`: regression test —
non-UUID refs (`"tumbly-haus-creative"`, `"not-a-uuid"`, `""`) resolve
to `null` without a query error.
## Verification
- `npx vitest run src/__tests__/companies-service.test.ts` — 12/12 pass
(new test included, embedded-postgres suite).
- Manual: `curl -i /api/companies/not-a-uuid` → 404 (was 500); `curl -i
/api/companies/<real-uuid>` → 200 unchanged.
## Risks
- Low. Pure input-validation guard on one read path; UUID lookups are
byte-for-byte unchanged. Only behavioral shift is 500→404 for refs that
could never have matched a row.
## Model Used
Claude Fable 5 (`claude-fable-5`, Anthropic) via Claude Code — diagnosis
from server logs, patch, and test authored with extended thinking and
tool use; human-reviewed and submitted by @christianlappin.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links
- [x] My branch name describes the change and contains no internal
ticket id
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes (n/a —
no doc references this error path)
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green (pending first CI run)
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(pending)
- [x] I will address all Greptile and reviewer comments before
requesting merge
|
||
|
dmndbrp-ossandCoder
|
0db8480b19 |
fix(SAG-2595): land updatedSince issues-list filter on master (#9050)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The issues-list REST endpoint (`GET /api/companies/:companyId/issues`) backs the digester and other pollers that ask "what changed since last time". > - The service layer supports rich filters, but there was no `updatedSince` filter — so every routine fire re-read the full backlog instead of just the delta. > - A prior commit added this filter, but it was never merged to `master`; it only ran in production because a feature branch happened to be the live checkout, and the behavior vanished when that directory was repurposed. > - This pull request re-lands just the `updatedSince` filter (route param parse + validation, service `IssueFilters` field, and the `updatedAt` predicate) as a single-purpose change. > - The benefit is that pollers can request only issues updated after a timestamp, and the fix now lives durably on `master` instead of a transient checkout. ## Linked Issues or Issue Description No public GitHub issue exists; describing inline per the bug report template. **What happened** `GET /api/companies/:companyId/issues` ignores an `updatedSince` query parameter, so consumers (e.g. the digester and other pollers) cannot request only the delta since a prior poll and must re-read the whole backlog on every fire. **Expected behavior** Passing `updatedSince=<ISO 8601 timestamp>` returns only issues whose `updatedAt` is strictly after that timestamp; a malformed value returns `400`. **Steps to reproduce** 1. Call `GET /api/companies/:companyId/issues?updatedSince=<a future ISO 8601 timestamp>`. 2. Observe the endpoint returns the full backlog instead of an empty list (the parameter is silently ignored). ## What Changed - `server/src/routes/issues.ts`: parse the `updatedSince` query param, return `400` for a non-parseable timestamp, and pass it into `svc.list()`. - `server/src/services/issues.ts`: add `updatedSince?: string` to `IssueFilters` and, when present and valid, add a `gt(issues.updatedAt, since)` condition to the list query. - `server/src/__tests__/issue-list-updatedsince-filter-routes.test.ts`: new route+service coverage — future timestamp returns 0 issues, a past timestamp returns only the delta, and a malformed timestamp returns 400. ## Verification - `pnpm vitest run src/__tests__/issue-list-updatedsince-filter-routes.test.ts` — 3/3 pass. - `pnpm vitest run src/__tests__/issue-list-assignee-filter-routes.test.ts` — 5/5 pass (regression check on the sibling filter path). - `tsc --noEmit` on `server/` — no new errors introduced (pre-existing unrelated `plugin-sdk` build errors on `master` are untouched). ## Risks Low risk. Purely additive: the new filter only takes effect when `updatedSince` is supplied, so existing callers that omit it are unaffected. Invalid timestamps fail fast with `400` rather than silently returning all rows. ## Model Used Claude — `claude-sonnet-4-6` (implementation) with `claude-opus-4-8` review/merge-gate; tool use + code execution 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 - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] 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 — no UI change) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (in progress) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (in progress) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Coder (Claude) <coder-claude@paperclip.ing> |
||
|
|
c5574599b1 |
fix(routines): exclude assignee configuration from detail responses (#9818)
## Thinking Path > - Paperclip is the open-source control plane people use to manage AI agents for work. > - Routines are the subsystem that schedules recurring work and returns routine detail to authorized company actors. > - Routine detail embedded the complete assignee database row even though its shared contract requires only assignee identity. > - That full row can contain protected adapter and runtime configuration, including environment bindings. > - The service boundary should project only the fields the routine contract actually needs. > - This pull request replaces the full-row query with a company-scoped identity projection and adds sentinel-based regression coverage. > - The benefit is useful routine detail without exposing protected assignee configuration. ## Linked Issues or Issue Description No public issue exactly tracks this service-level exposure. - Related prior PR: Refs #4967, an older route-level redaction approach with broader changes and no focused routine serialization test. - Related closed PR: Refs #5144, an unmerged prior implementation of the same identity-projection approach. - Related agent-route hardening: Refs #8779; that work covers direct agent responses, while this PR removes protected fields from the routine embed itself. Bug details: - Actual behavior: `GET /api/routines/{routineId}` could serialize the complete assignee row, including protected adapter/runtime configuration. - Expected behavior: routine detail exposes only the assignee identity required by `RoutineDetail`, including its derived `urlKey`. - Reproduction: assign an agent with sentinel-only protected configuration to a routine, retrieve routine detail, and inspect key presence or serialize the response; no production value is needed or recorded. - Version/commit reproduced: upstream `master` immediately before this PR. - Deployment mode: service-level embedded Postgres test; the vulnerable serializer is shared by supported deployments. ## What Changed - Added a company-scoped assignee summary query in `server/src/services/routines.ts` that selects only `id`, `name`, `role`, and `title`, then derives the non-sensitive `urlKey` from the name. - Updated `getDetail()` to use that projection instead of selecting the complete agent row. - Added focused negative and positive identity assertions, including the derived `urlKey`, in `server/src/__tests__/routines-service.test.ts`. - Audited routine list/detail serialization and broader embedded-agent query sites; routine list exposes only `assigneeAgentId`, while other agent embeds use explicit projections or authorized agent endpoints. ## Verification - `pnpm exec vitest run server/src/__tests__/routines-service.test.ts` — 57/57 passed. - Focused sentinel regression test — passed. - `pnpm -r typecheck` — passed. - Server, UI, and CLI builds — passed; UI gzip-size completion used a 4096 MB Node heap. - `git diff --check` — passed. - Full `pnpm test:run` — 2,699 passed, 1 skipped, 9 failed in untouched tests. The failures reproduce outside this change and are limited to local-adapter `nohup`/PTY behavior, macOS `/tmp` versus `/private/tmp` normalization, and one workspace-runtime auto-port fixture. ## Risks - Low compatibility risk: the returned shape now matches the existing shared `RoutineDetail` contract. - A consumer relying on undocumented protected agent fields inside routine detail will stop receiving them. - No schema, migration, deployment, credential, or production-secret changes are included. - Rollback is a single commit revert, but reverting would restore the exposure. > This is security hardening for the already-shipped routines subsystem; `ROADMAP.md` marks Scheduled Routines complete, and this PR does not add or duplicate roadmap feature work. ## Model Used - OpenAI GPT-5 via Codex, with repository search, local code execution, tests, TypeScript typechecking, builds, Git, and GitHub API use. The runtime does not expose a more granular snapshot ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and relevant tests pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (no documentation change is required for this contract-preserving security fix) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green 5 with no open P2s, recommendations, or follow-ups/- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: ClawdeBot <clawdebot@Mac-mini-de-ClawdeBot.local> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DirkandClaude Opus 4.8
|
8b1483e601 |
fix(ui): render board approval payload prose as markdown (#9817)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The approval queue is how agents surface decisions that need a human, so the approval card is often the only thing an operator reads before approving or rejecting > - Agents author those payloads in markdown, because markdown is what they produce everywhere else in the product > - `ApprovalPayload.tsx` renders the four prose fields of a board approval as bare text nodes, while `CommentThread` on the same page renders through `MarkdownBody` — same authors, two different renderers > - So the operator sees literal `##`, `**bold**`, backticks and `[link](url)` in the payload, and correctly formatted text in the comments directly below it > - This pull request routes those four fields through the existing `MarkdownBody` component > - The benefit is that the highest-stakes text in the product becomes readable, with no new dependency and no schema change ## Linked Issues or Issue Description Refs #4911 — prior art, see the note at the bottom of this description. No open issue covers this, so per (B) here is the bug report: **What happened:** On a board approval, the `summary`, `recommendedAction`, `nextActionOnApproval` and `risks` fields display raw markdown source. Headings appear as literal `##` mid-paragraph, inline code keeps its backticks, links show as `[text](url)`, and both levels of a nested bullet list collapse into one run-on paragraph. **What was expected:** The same rendering the comment thread further down the same page already gives, since both are agent-authored markdown. **Steps to reproduce:** Open any `request_board_approval` whose `summary` contains markdown — headings, a nested list, code spans or links. **Where:** `ui/src/components/ApprovalPayload.tsx`, `BoardApprovalPayloadContent`. ## What Changed - `ui/src/components/ApprovalPayload.tsx`: import `MarkdownBody` and render `summary`, `recommendedAction`, `nextActionOnApproval` and each `risks` entry through it instead of `<p>` / `<span>` text nodes. `MarkdownBody` defaults `softBreaks` to `true`, which is the same behaviour `CommentThread` opts into explicitly, so paragraph handling matches the comments. - `stripLeadingListMarker`: risks already render inside a custom bullet row, so an authored leading `-` / `*` / `•` would nest a second bullet inside the first. One leading marker is stripped per entry. - The risk bullet dot gains `shrink-0` so it keeps its shape next to block-level markdown content. - `title` stays plain text — it is a one-liner and markdown there is noise. - `proposedComment` stays a verbatim `<pre>` block — it is draft text intended to be posted elsewhere, so it must not be reinterpreted. - `ui/src/components/ApprovalPayload.test.tsx`: tests for markdown rendering in all four fields, the leading-list-marker strip, and that `title` and `proposedComment` remain verbatim. ## Verification - `npx vitest run ui/src/components/ApprovalPayload.test.tsx` — 5 passed. - `npx vitest run ui/src/components/ApprovalPayload.test.tsx ui/src/components/CommentThread.test.tsx` — 12 passed, confirming the shared `MarkdownBody` path is not disturbed. - Manual, measured rather than eyeballed: I ran a patched build in a throwaway container beside an unpatched one and pointed both at the same real approval payload, then counted nodes in the rendered DOM. | | unpatched | patched | |---|---|---| | `.paperclip-markdown` nodes | 0 | 3 | | raw backticks in visible text | yes | no | | rendered `h2` | 0 | 5 | | rendered `li` | 0 | 16 | ## Risks Low, and confined to the board approval card. - Rendering scope widens from text to markdown on four fields. A payload that contains markdown-significant punctuation but was authored as prose could render differently than before. This is the intended change, and it matches how the same author's text is already rendered in comments on the same page. - `stripLeadingListMarker` removes one leading list marker per risk entry. A risk that genuinely begins with a literal hyphen followed by a space loses that hyphen. Chosen over the alternative of a visible double bullet on the common case. - No schema change, no migration, no new dependency. `MarkdownBody` is already used elsewhere in the same directory. ## Model Used Claude Opus 4.8 (`claude-opus-4-8`), via Claude Code, with extended thinking and tool use (repository search, file editing, local test execution, and headless-browser DOM measurement of the before/after renders). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes — no docs describe this rendering behaviour, so there was nothing to update - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green — pending first CI run on this PR - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending first review - [x] I will address all Greptile and reviewer comments before requesting merge --- ### On the prior PR @alxhrzg opened #4911 for this same bug first, and reached the same conclusion I did: route the four fields through `MarkdownBody`. Credit for spotting it and for the diagnosis goes there. That PR has been conflicting against base and untouched since May. Rather than let the fix sit, this PR reapplies the idea on current `master` and adds what #4911 was missing: test coverage, the nested-bullet fix for `risks`, and the `shrink-0` on the bullet dot. I could not push to #4911 directly as it is on another contributor's fork. @alxhrzg, if you would rather finish #4911 yourself, I am happy to close this and hand over the tests and the two risk-row fixes for you to take across. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
LeeJandLeeJ
|
5521d768b2 |
fix(claude-local): avoid root-only skip permissions failure (#9463)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Claude local is one of the adapter paths that lets operators run Claude Code through a local Paperclip runtime. > - Claude Code rejects `--dangerously-skip-permissions` when the process is running as root or through sudo. > - Local/self-hosted Paperclip deployments may run inside root-owned Docker/runtime processes, so the Claude local adapter can fail before it reaches the actual runtime/auth condition. > - Paperclip already uses a curated `--allowedTools` list instead of `--dangerously-skip-permissions` for remote Claude targets. > - This pull request applies the same safer permission strategy to local root processes while preserving existing local non-root and remote behavior. > - The benefit is clearer, safer Claude local diagnostics/execution in containerized setups without widening permissions beyond the existing explicit tool allowlist. ## Linked Issues or Issue Description No directly matching public issue or PR found. Bug description: - **Problem:** `claude_local` can fail its local probe/execution path when Paperclip runs from a root-owned local container/runtime because Claude Code refuses `--dangerously-skip-permissions` under root/sudo. - **Actual behavior:** The adapter may fail immediately with Claude's root/sudo guard before validating the real Claude runtime/auth state. - **Expected behavior:** Local root processes should use the same explicit allowlist strategy Paperclip already uses for remote targets, while local non-root behavior remains unchanged. - **Environment:** Local/self-hosted Docker or container-style Paperclip runtime where the app process UID is `0`. Related but different: #4926 covers MCP config propagation for the Claude local adapter, not the root/sudo permission flag behavior fixed here. ## What Changed - Added root-aware permission argument selection for the Claude local adapter. - Preserved current local non-root behavior: `--dangerously-skip-permissions` is still used when allowed. - Preserved current remote behavior: remote targets continue using explicit `--allowedTools`. - Changed local root behavior to use the explicit `--allowedTools` list instead of `--dangerously-skip-permissions`. - Threaded process UID awareness through Claude local probe and execution paths. - Added unit coverage for skip-disabled, remote, local non-root, local root, and UID-unavailable behavior. ## Verification ```sh ./node_modules/.bin/vitest run --config g15-vitest-claude-local.config.mjs \ packages/adapters/claude-local/src/server/permissions.test.ts ``` Result: ```text 1 file passed 8 tests passed ``` ```sh pnpm --filter @paperclipai/adapter-claude-local typecheck ``` Result: ```text @paperclipai/adapter-claude-local typecheck passed ``` Additional local smoke: - Ran a disposable root-container Claude adapter diagnostic against this patch. - The diagnostic no longer fails with Claude's root/sudo `--dangerously-skip-permissions` error. - It proceeds to the actual environment-specific Claude auth/runtime result. - No credentials, tokens, hostnames, private paths, or internal Paperclip issue references are included in this PR. Public duplicate checks performed: ```sh gh pr list --repo paperclipai/paperclip --state open --search 'claude local root permissions dangerously skip permissions allowedTools' gh issue list --repo paperclipai/paperclip --state open --search 'claude local root permissions dangerously skip permissions allowedTools' ``` ## Risks Low-to-medium risk adapter behavior change: - Local root Claude runs will now use explicit `--allowedTools` rather than broad skip-permissions behavior. - That is intentionally safer, but an environment depending on broader implicit tool access under root may now need the adapter allowlist to include any required tools. - Local non-root behavior is unchanged. - Remote behavior is unchanged. - No database migrations, API contract changes, or UI changes. > 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 `gpt-5.5` via Hermes Agent, with shell/file/tool use for repository inspection, patching, local verification, and GitHub CLI operations. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: LeeJ <elJayAdvisor@users.noreply.github.com> |
||
|
Nicky LeachandClaude Opus 4.8
|
04bf7a6ab5 |
feat(observability): instrument stage.sync host steps and home the agent process span (#11301)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - It runs each agent in a remote sandbox and emits OpenTelemetry spans for the sandbox bring-up and the run. > - A real trace showed two gaps. `stage.sync` had about 3 seconds of unattributed host work before its `pack` span. The persistent agent process showed a `sandbox.exec` span that outlived its parent by about 50 seconds. > - The gaps hide real cost and make the trace read as a sequencing bug, so an operator cannot see where startup time goes. > - This pull request wraps the two pre-`pack` host steps in their own spans. It also homes the long-lived process in a run-scoped `sandbox.agentProcess` span. > - The benefit is that startup time is fully attributed and the process reads as a resource that overlaps the turn, not a child that outlives its parent. ## Linked Issues or Issue Description No public issue exists. This is an enhancement to existing telemetry. It is described inline below, following `.github/ISSUE_TEMPLATE/enhancement.yml`. Prior related work: the merged PR #10999 added the run-time wrapper spans and the telemetry data-contract section this PR extends. **What existing behavior does this improve?** The sandbox bring-up and run OpenTelemetry trace. It closes two attribution gaps in that trace. **Subsystem affected** Observability for sandbox execution. The code lives in `packages/adapter-utils`. The span contract lives in `packages/shared/src/telemetry`. **Current behavior** `stage.sync` opens a `pack` span, but the git enumeration and the baseline content-hash walk that run before `pack` have no span, so about 3 seconds read as a gap. On the streamed process-session path the agent process launches fire-and-forget inside the ~2.3 second `bridge.process-session` bring-up step, so its `sandbox.exec` span parents to that step and then runs about 50 seconds. The child dangles past its parent and overlaps `agent.turn`. **Proposed behavior** Wrap the two pre-`pack` host operations in `snapshot.git` and `snapshot.baseline` spans under `stage.sync`. Wrap the streamed launch in a run-scoped `sandbox.agentProcess` span that parents to the live run root (`task.run` at launch). **Reason and benefit** Startup time is fully attributed. The long-lived process reads as a resource that overlaps the sibling `agent.turn`, not a mis-parented child. **Breaking changes** None. The spans are opt-in and export only when an OTLP endpoint is configured. The span seam is a no-op when no runner is injected. No first-party telemetry event changes. ## What Changed - `sandbox-managed-runtime.ts`: add `snapshot.git` and `snapshot.baseline` spans around the git enumeration and the baseline content-hash walk, nested under `stage.sync`, through a shared `runStepSpan` helper that `pack` now also uses. - `execution-target.ts`: wrap the fire-and-forget streamed launch in a run-rooted `sandbox.agentProcess` span, so it parents to the live run root and holds the inner `sandbox.exec`. The `.then`/`.catch` chain became try/catch inside the span callback, with identical frame-ingestion behavior. - `packages/shared/src/telemetry/README.md`: update the span table and the parenting prose. Add `snapshot.git`, `snapshot.baseline`, `pack`, and `sandbox.agentProcess`, and document the intended `sandbox.agentProcess` / `agent.turn` overlap. - Tests: update the executor span-tree test (`childNames` and parent assertions), update the `sandbox-managed-runtime` span-set and nesting tests, and add two `execution-target-sandbox` tests (the launch opens `sandbox.agentProcess`; it parents to the run root, not the bring-up step). ## Verification - Run `npx vitest run` on the three affected test files. Result: 174 tests pass. This includes the updated executor span-tree test and the new `sandbox.agentProcess` open and parenting tests. - Run `tsc --noEmit` in `packages/adapter-utils`. Result: no errors in the changed source or test files. - The full 37-test streamed process-session suite passes unchanged. This confirms the try/catch restructure preserves frame delivery and exit/error behavior. - Pre-existing and unrelated to this PR (present on `master`): `tsc` errors in `execute.ts` / `execute.test.ts` / `remote-spawn-smoke.test.ts` (`onAgentStderr` / `spawnCwd`), and a `check:forbidden-tokens` failure from internal `PAP-###` ids in `ui/src/components/IssueRecoveryActionCard.test.tsx`. This PR does not touch those files, and its own diff is token-clean. ## Risks Low. The change adds instrumentation on the opt-in span path and does not change control flow on the default path. The one production restructure is the streamed launch, which stays fire-and-forget, so bring-up does not block on it. Only the streamed path gains `sandbox.agentProcess`; the legacy poll path launches the process detached and has no host-side long-lived span to home. ## Model Used Anthropic Claude Opus 4.8 (`claude-opus-4-8`), about 200K-token context, agentic tool use through Claude Code. The trace was reviewed through the Honeycomb MCP. The code was written and tested with the model. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>canary/v2026.812.0-canary.14 |
||
|
Devin Foley
|
61a5b7c6f9 |
fix(ui): stop the selection ping-pong on archived company URLs (#11300)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The UI keeps a selected company in `CompanyProvider` with two writers: a bootstrap effect that repairs invalid selections, and a Layout route-sync effect that selects the company the URL prefix names > - The route-sync matches the URL against the full company list (archived included), while the bootstrap resolver only accepted companies from the sidebar-filtered non-archived list > - On any archived company's URL the two effects overwrite each other's selection in a synchronous loop until React throws error #185 ("Maximum update depth exceeded") and unmounts the root to a blank page — armed by remembered last-visited paths, back/forward navigation, or bookmarks, on first load and client navigation alike > - This pull request makes an already-selected company only need to exist, keeping the sidebar filter for fresh-boot resolution where no explicit selection exists > - The benefit is that archived company URLs render instead of blanking the entire app ## Linked Issues or Issue Description No existing issue. Description follows the bug template: **What happened?** Opening (or back-navigating to) a URL whose company prefix belongs to an archived company blanked the whole app with `Minified React error #185`. Console in dev mode: "Maximum update depth exceeded. This can happen when a component calls setState inside useEffect…". A workspace whose first/seeded company was archived hit this on every load of its remembered URL. **Expected behavior** An archived company's URL renders its pages (the company still exists and its API routes serve data). The sidebar simply does not feature archived companies, and fresh boots still land on a non-archived company. **Steps to reproduce** 1. Create two companies; archive one (`PATCH /api/companies/:id` with `status: "archived"`). 2. Navigate to `/{archivedPrefix}/dashboard` — direct load or client-side back-navigation. 3. Before this fix: React #185 and an unmounted blank page (reproduced deterministically by the new e2e test). ## What Changed - `ui/src/context/CompanyContext.tsx`: `resolveBootstrapCompanySelection` keeps an explicitly selected company that exists in the full company list; stored-id and default resolution still prefer sidebar (non-archived) companies. - `ui/src/context/CompanyContext.test.tsx`: resolver keeps an archived-but-existing selection; a truly deleted selection is still replaced. - `tests/e2e/archived-company-url.spec.ts`: end-to-end regression driving both field shapes (direct load and back-navigation onto an archived company URL); it failed with the exact #185 console errors before the fix and passes after. ## Verification - `pnpm vitest run src/context …` in `ui/` — 122 tests pass (includes the new resolver cases). - `npx playwright test --config tests/e2e/playwright.config.ts archived-company-url` — fails before the fix (captured "Maximum update depth exceeded" console errors), passes after. - `pnpm typecheck` in `ui/` — clean. ## Risks Low risk. The only behavioral change is that a selection naming an archived-but-existing company survives the bootstrap repair — previously that state was unreachable without crashing. Boots with no valid selection behave exactly as before (non-archived preferred), covered by the existing and new resolver tests. ## Model Used - Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code CLI with extended thinking and tool use (code search, edit, test execution, Playwright-driven crash reproduction). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting mergecanary/v2026.812.0-canary.13 |
||
|
Constantineandcucurigoo
|
276730d63e |
fix(server): recognize cross-package Zod errors (#10168)
## Thinking Path > - Paperclip validates API request bodies with Zod and converts validation failures into client errors. > - The global error handler recognized Zod failures with `instanceof ZodError`. > - Monorepo dependency layouts can provide more than one installed Zod module instance. > - A valid Zod error from another instance fails that identity check and falls through as HTTP 500. > - This pull request keeps the native path and adds a narrow structural fallback for named Zod errors with an issues array. > - The benefit is stable HTTP 400 validation semantics regardless of package-instance identity. ## Linked Issues or Issue Description Related but not duplicate: Refs #6908. That PR catches `instanceof ZodError` inside validation middleware and returns 422; it does not cover errors created by a second Zod module instance, which is the reproduced failure here. **What happened?** An invalid `POST /api/issues/:id/work-products` payload raised a real Zod validation error but returned HTTP 500 because the error came from a different Zod package instance. **Expected behavior** All genuine Zod validation failures return HTTP 400 with validation details, independent of module identity. **Steps to reproduce** 1. Submit a work-product body missing the required `provider`, `externalId`, and `url` fields. 2. Ensure the route schema is resolved from a different installed Zod instance than the server error handler. 3. Observe HTTP 500 before this fix. 4. Observe HTTP 400 after this fix. **Environment** - Paperclip base: `14f20be92b86a49ff2c35495e5b0fa4d719998ef` - Deployment: self-hosted, built from source - Access context: board API - Adapter scope: not adapter-specific - [x] I searched open PRs for `ZodError`, validation errors, and work-product validation and linked related work above. ## What Changed - Add a narrow `readZodIssues` helper that accepts native Zod errors or structurally valid cross-package Zod errors. - Preserve existing HTTP 400 response shape and structured error context. - Add a regression for a Zod error object from another module instance. ## Verification - `pnpm exec vitest run server/src/__tests__/error-handler.test.ts` - `pnpm --filter @paperclipai/server typecheck` - Full upstream CI test/build/e2e matrix passed. - Local post-deploy smoke returned HTTP 400 for the previously failing invalid work-product payload. ## Risks - A deliberately thrown object named `ZodError` with an `issues` array will be treated as a client validation failure. The effect is limited to returning HTTP 400 instead of 500; no authorization or persistence behavior changes. - No schema or migration changes. > This is a bug fix, not roadmap feature work. ## Model Used OpenAI Codex `gpt-5.6-sol`, with tool use, code execution, repository inspection, and independent read-only review agents. ## 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 linked related public work and described the bug in-PR following the bug template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have considered documentation; no user-facing documentation change is required - [x] I have considered and documented risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: cucurigoo <cucurigoo@users.noreply.github.com>canary/v2026.812.0-canary.12 |
||
|
|
e31951a17d |
feat: Claude agent setup-token login in a sandbox (#11286)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Claude agents that run in a remote sandbox need a safe in-product login path > - The existing host login route cannot open a pseudo-terminal inside that sandbox > - The login flow must protect the browser code, the login URL, and the OAuth token at every step > - This pull request adds the parser, the runner, a Daytona pseudo-terminal transport, and a guarded, owner-bound session route behind an injectable transport > - The route stays inert in the default build and fails closed until a sandbox provider binds the live transport > - The benefit is a company-scoped setup-token flow with one-time secret delivery, redaction, and fail-closed transport checks, ready for a later staged production rollout ## Linked Issues or Issue Description **Agent or provider** Claude Code setup-token login for sandbox agents. **Why this adapter is useful** Sandbox agents need a supported way to sign in without host credentials. An authorized owner completes the browser step and receives the token one time. **How the agent is invoked** When a sandbox provider binds the injectable transport, the server starts `claude setup-token` through a sandbox pseudo-terminal, sends the browser code to the matched prompt, and returns the token through the guarded session route. The default build does not bind the transport. In that state the start route fails closed with a fixed no-secret `503`. It does not start a process and it does not hold a sandbox lease. **Additional context** The transport is injectable, so each sandbox provider binds its own pseudo-terminal. This pull request adds the Daytona transport but does not bind it in the production server. A production wiring needs a lease manager, a live pseudo-terminal factory, a durable token store, and its own security review. The route keeps secrets out of logs, activity details, errors, telemetry, and non-owner responses. ## What Changed - Add strict parsers for the setup-token URL, the prompt, and the success token. - Add a login runner that drives the `claude setup-token` command through a pseudo-terminal. - Add the Daytona pseudo-terminal transport and the sandbox plugin wiring. - Add a company-scoped, owner-bound login session service with rate limits, a reaper, cleanup, and one-time token delivery. - Add the guarded session routes at `/agents/:id/setup-token-login-sessions/*` behind an injectable transport. The routes become the live login path only when a provider binds the transport. - Keep the start route fail-closed in the default build. It returns a fixed no-secret `503` and it does not bind `setupTokenLogin`. - Keep the existing host route `POST /agents/:id/claude-login` in place. This pull request does not replace it. - Keep confidential responses behind a fail-closed TLS transport guard with `Cache-Control: no-store`, and extend redaction for the new fields. - Export the parser and the runner from the Claude local server entry, and document the new session routes in the OpenAPI spec. ## Verification - `pnpm --filter @paperclipai/server exec vitest run setup-token-route setup-token-session` - `pnpm --filter @paperclipai/adapter-claude-local exec vitest run` - `pnpm --filter @paperclipai/server run typecheck` - Confirm that the pull request checks pass on GitHub. ## Risks - Low user-facing risk on merge. The default build does not bind the transport, so the production start route stays fail-closed with a `503`. The merge does not change the production login behavior. - When a provider later binds the transport, the flow starts a live sandbox process and holds a short-lived in-memory secret. Cleanup must stop the child before it releases the sandbox lease. - The transport guard fails closed when the deployment does not provide a trusted TLS path. A wrong proxy allowlist can block a valid request. - The production wiring is out of scope. It needs a lease manager, a live pseudo-terminal factory, a durable token store, and its own security review before the server binds `setupTokenLogin`. ## Model Used Anthropic Claude Opus 4.8 assisted the implementation. It used extended reasoning, code execution, repository tool use, and a 200,000-token context window. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (the OpenAPI spec covers the new session routes; no user-facing documentation needs changes) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>canary/v2026.812.0-canary.11 |
||
|
|
0ee0543e5b |
feat(ui): refine the chat-style task workflow (#11263)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The issue detail view is the main place where people read work and guide agents. > - The chat-style task view needs clear messages, controls, properties, and document feedback. > - Dense metadata and disconnected controls make active work harder to scan. > - This pull request refines the existing chat-style task workflow across desktop and mobile layouts. > - The benefit is a clearer issue thread with faster access to the controls that guide work. ## Linked Issues or Issue Description **What existing behavior does this improve?** The change improves the chat-style issue detail view that was introduced in [#10606](https://github.com/paperclipai/paperclip/pull/10606) and expanded in [#10707](https://github.com/paperclipai/paperclip/pull/10707). **Current behavior** The issue thread spreads task controls across the page. Agent-turn metadata competes with the message content. Document annotation comments use inline placement that limits the document reading area. The mobile composer can overlap the bottom navigation. **Proposed behavior** The issue view keeps the thread focused on message content. It moves supporting controls into the properties area, adds searchable assignment, restores the sub-task tree, docks document comments in a side gutter, and keeps the mobile composer clear of navigation. **Reason and benefit** People can scan active work faster and find task controls without leaving the issue. The layout also gives documents and mobile conversations more usable space. **Breaking changes** None. The change updates presentation and interaction behavior in the existing issue UI. ## What Changed - Refined task-chat message spacing, metadata, agent bubbles, and composer alignment. - Added searchable assignment and restored sub-task navigation in the properties pane. - Moved document annotation comments into a right-side gutter. - Kept the mobile composer above the auto-hiding bottom navigation. - Added and updated focused component tests for the changed interactions. ## Verification - `pnpm check:token-gates` - `TZ=UTC pnpm --filter @paperclipai/ui exec vitest run src/components/InlineEntitySelector.test.tsx src/components/IssueDocumentAnnotations.test.tsx src/components/IssueProperties.test.tsx src/components/TaskChatThread.test.tsx src/pages/IssueDetail.test.tsx` - The token gates report 3/3 clean. - The focused test run passes 124 tests in 5 files. - Visual snapshot baselines were not updated. This follows the `doc/design/DECISION-SHEET.md` entry "Per-change snapshot verification demoted to dormant (Jul 13 2026)." ## Risks - The changes affect several related issue-detail layouts. A browser review should cover desktop and mobile widths before merge. - The monitor-row test formats time in the host timezone. The verification command sets `TZ=UTC` to match CI. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex with GPT-5. The context-window size is not exposed in this environment. The model used reasoning, repository tools, code execution, and GitHub tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Paperclip <noreply@paperclip.ing>canary/v2026.812.0-canary.10 |
||
|
Devin Foley
|
ff5fd62d07 |
Resolve the environment secret companyId context on first save (#11291)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Environments give agent runs an execution target, and their env vars can carry secrets, so writes need a company scope for secret bindings > - `PATCH /environments/:id` resolves that scope from a query param, existing bindings, or a single-membership actor — and fails closed otherwise > - A fresh environment has no bindings yet, so an admin whose memberships cannot pin one company gets a 422 on the very first env var save and can never create the first binding > - #11200 made this reachable from the UI: it opened the envVars-only edit on the managed sandbox row, but the UI never sends the company it already knows > - This pull request sends the page's selected company on environment updates and adds a server fallback to the instance's only company > - The benefit is that the first env var save works on fresh environments, while multi-company ambiguity still fails closed ## Linked Issues or Issue Description Refs #11200 **What happened?** On a fresh platform-managed sandbox environment, the first "Save environment variables" in the UI fails with HTTP 422: "Environment secret management requires a companyId context during the instance-scoped transition." The same failure hits any environment update that touches `envVars` or `config` when the environment has no secret bindings yet and the actor's memberships do not name exactly one company (for example an instance admin provisioned without a membership row, or a user in two companies). **Expected behavior** The save succeeds. The client tells the server which company scopes the secret bindings, and on a single-company instance the server can resolve the only possible scope by itself. **Steps to reproduce** 1. Provision a platform-managed sandbox environment (managed-config `environments` entry) so the row exists with no secret bindings. 2. Sign in as an instance admin whose memberships do not resolve to exactly one company. 3. Open Environments, edit the managed row, add an env var, and save. 4. The PATCH returns 422 with the companyId-context error. ## What Changed - `ui/src/api/environments.ts`: `environmentsApi.update` accepts an optional `companyId` and sends it as the `companyId` query param the server already reads. Renamed the `customImageCompanyQuery` helper to `companyIdQuery` since it now serves plain updates and probes too. - `ui/src/pages/CompanyEnvironments.tsx`: both the managed envVars-only save and the general environment save pass `selectedCompanyId`. - `server/src/routes/environments.ts`: `resolveEnvironmentSecretContextCompanyId` falls back to the instance's only company when exactly one exists, mirroring the existing fallback in `resolveCustomImageCompanyId`. Multi-company instances still fail closed. - Tests: three new route tests (explicit query context for a multi-company actor, single-company-instance fallback for a membership-less admin, fail-closed regression on a multi-company instance), and updated the SSH-probe and probe-ambiguity tests for the new fallback. ## Verification - `cd server && pnpm vitest run src/__tests__/environment-routes.test.ts` — 78 pass. - `cd ui && pnpm vitest run src/pages/CompanyEnvironments.test.tsx` — 22 pass. - Full `server` and `ui` vitest suites and `tsc --noEmit` on both packages pass locally. - Manual: on a managed instance, edit the managed sandbox environment, add an env var, save. Before: 422. After: the save persists and the PATCH carries `?companyId=`. ## Risks - Low risk. The explicit query param path already existed on the server; the UI now uses it. - Behavioral shift: on single-company instances, environment secret-context resolution (including `POST /environments/:id/probe`) now resolves the only company instead of returning no context. That lets probes resolve secret-backed config where they previously returned a 422 asking for an explicit companyId. Multi-company instances keep the fail-closed behavior, covered by a regression test. - Self-hosted single-company instances gain the same first-save fix; no schema or config changes. ## Model Used Claude Fable 5 (`claude-fable-5`, Anthropic) via Claude Code — extended thinking, agentic tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Nicky LeachandPaperclip
|
f1931d0e14 |
test(server): fix flaky workspace-busy retry-row read race (#11293)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server heartbeat system records each agent run and its retry state > - A workspace-busy deferral cancels one run before it inserts the scheduled retry row > - The test helper can return after the cancel write and before the retry-row insert > - A direct read can then return no row and fail a valid retry assertion > - This pull request makes presence reads wait for the retry row > - The benefit is stable test coverage without a production behavior change ## Linked Issues or Issue Description Refs: #10806 **What happened?** The workspace-busy test read the retry row after the first deferral write. The helper returned before the scheduled-retry insert completed. The read then returned no row and failed the retry assertions. **Expected behavior** The test must wait until the scheduled-retry row exists before it checks retry-row fields. The production write order must stay unchanged. **Steps to reproduce** 1. Add a 300 ms delay between the deferral writes. 2. Run `server/src/__tests__/heartbeat-workspace-busy.test.ts`. 3. Observe failures at retry-row presence checks. 4. Add the bounded polling helper. 5. Run the test file again and observe that all presence checks pass. **Paperclip version or commit** Commit `d9b6e8a6e62b9b56919fc9c52d294e8ac569f70f`. **Deployment mode** Local test run from source. ## What Changed - Add `waitForRetryRun`, which polls for the retry row with a 10 second timeout and a 50 millisecond interval. - Use the helper at every test site that reads a retry row after deferral. - Keep direct reads at absence assertions. - Keep production code unchanged. ## Verification - Injected a temporary 300 millisecond delay between the two production writes and reproduced the five presence-site failures. - Applied the helper with the delay and passed the test file 15 out of 15 times. - Removed the temporary production delay. - Ran the changed test file 25 consecutive times with 0 failures. - Ran TypeScript checks for the changed test file with no errors. ## Risks Low risk. This pull request changes test code only. The helper has a bounded timeout. Production behavior and retry-row assertions remain unchanged. ## Model Used OpenAI Codex, GPT-5, reasoning mode, tool use, and code execution. The runtime does not expose the context window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Devin Foley
|
c57c0f7498 |
fix(sandbox-providers): accept bsdtar listings in the syncOut tarball confinement check (#11289)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Sandbox providers (Daytona, Kubernetes) sync run results back to the host with a sandbox-authored tarball > - Before extraction, a confinement check parses the host `tar -tvf` listing and fails closed on unparseable lines > - The parser only understands the GNU tar listing dialect; macOS ships bsdtar, whose ls-style listing never matches > - Every sandbox syncOut on a macOS host therefore aborts with "refusing tarball with an unparseable entry listing", and the run fails at copy-back > - This pull request teaches the parser both dialects while keeping the fail-closed and traversal guarantees > - The benefit is that Daytona and Kubernetes sandbox runs work on macOS hosts, with no behavior change on Linux ## Linked Issues or Issue Description No existing issue. Bug description: **What happened?** On a macOS host, every Daytona sandbox run fails at syncOut. The adapter reports: `Daytona syncOut refusing tarball with an unparseable entry listing: -rw-r--r-- 0 daytona daytona 7560 Aug 11 21:43 AGENTS.md`. The Kubernetes provider has the same parser and fails the same way. **Expected behavior** The confinement check accepts a well-formed listing from the host tar, whichever dialect the host tar emits. It still rejects members that escape the extraction directory, and it still fails closed on lines it cannot parse. **Steps to reproduce** 1. Run Paperclip on macOS (system tar is bsdtar). 2. Configure an agent with the Daytona sandbox provider. 3. Trigger any run that syncs files back from the sandbox. 4. The run fails at syncOut with the unparseable-entry-listing error, because bsdtar prints `<perms> <links> <user> <group> <size> <Mon> <day> <time|year> <name>` while the parser expects the GNU `<perms> <owner>/<group> <size> <date> <time> <name>` shape. **Operating system** macOS (bsdtar 3.5.3). Linux hosts with GNU tar are unaffected. ## What Changed - Extracted the listing-line parse in both providers' `file-sync.ts` into an exported `parseTarVerboseListingLine` that accepts the GNU/busybox dialect and the bsdtar (libarchive) dialect. - The GNU shape now requires the slash-joined `<owner>/<group>` field. This keeps the two shapes mutually exclusive. Without it, a bsdtar line with numeric uid/gid satisfies the loose GNU pattern shifted by one field, which would hide a leading `../` from the traversal check. - Unparseable lines still fail closed. This includes device-node entries, whose size column is `major,minor` in both dialects. - Made the path-traversal fixture in the Daytona suite portable: GNU spells member renaming `--transform`, bsdtar spells it `-s`. - Added a Daytona test that refuses a sandbox-authored tarball carrying a symlink whose target escapes the extraction dir. - Added parser unit tests for both dialects (file, dir, symlink, hardlink, numeric owner, year-form dates, fail-closed lines) to both providers' suites. ## Verification - `pnpm test` in `packages/plugins/sandbox-providers/daytona`: 136/136 pass on a macOS host. On unpatched `master` the round-trip test fails there with the unparseable-entry-listing error. - `pnpm test` in `packages/plugins/sandbox-providers/kubernetes`: the new parser tests pass; no new failures against the `master` baseline on the same host. - `pnpm typecheck` passes in both packages. - CI runs the same suites on Linux/GNU tar and proves the GNU path is unchanged. ## Risks - Low risk. The GNU pattern is one token stricter (`<owner>/<group>` must contain `/`). GNU and busybox tar always print the slash-joined owner field, so accepted GNU listings are unchanged. - The bsdtar branch only widens acceptance on hosts that were failing 100% of syncOuts before, so no working deployment changes behavior. - The check still fails closed on anything neither pattern matches. ## Model Used Claude Fable 5 (`claude-fable-5`, Claude Code CLI, extended thinking + tool use). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Devin Foley
|
01112c350c |
fix(ui): always give respondWith a real Response in the sw fetch fallback (#11292)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The UI registers a service worker (`ui/public/sw.js`) with a
network-first fetch handler whose cache is an offline fallback
> - The fallback hands `event.respondWith` the result of
`caches.match(...)`, which resolves `undefined` on a cache miss — and in
the navigation branch, `caches.match("/") || offlineResponse` never uses
the fallback because `caches.match` returns a promise, which is always
truthy
> - When the network fetch rejects (server restart, deploy, brief
outage) and the cache misses, the browser fails the request with
`Uncaught (in promise) TypeError: Failed to convert value to
'Response'`, so navigation breaks outright instead of degrading to the
offline page
> - This pull request awaits the cache lookups and guarantees a real
`Response` on every path
> - The benefit is that brief server unavailability degrades to the
offline fallback instead of a dead navigation
## Linked Issues or Issue Description
No existing issue. Description follows the bug template:
**What happened?**
Navigating while the server was briefly unavailable (mid-restart)
produced `The FetchEvent for "…" resulted in a network error response:
the promise was rejected.` and `sw.js:1 Uncaught (in promise) TypeError:
Failed to convert value to 'Response'.` The navigation failed instead of
showing the offline fallback.
**Expected behavior**
A failed navigation serves the cached app shell when present, otherwise
the "Offline" 503 response. A failed asset fetch serves its cache entry
when present, otherwise a proper network-error response. `respondWith`
always receives a real `Response`.
**Steps to reproduce**
1. Load the app so `sw.js` is active; ensure `/` is not in the service
worker cache (fresh cache version).
2. Restart or stop the backend.
3. Navigate to any page: the fetch rejects, `caches.match` misses, and
the browser logs the conversion TypeError with a failed navigation.
## What Changed
- `ui/public/sw.js`: the fetch fallback awaits `caches.match(...)` and
returns the "Offline" 503 for navigations and `Response.error()` for
assets when the cache misses.
- `ui/src/lib/sw-offline-fallback.test.ts`: evaluates the real `sw.js`
in a sandboxed scope and covers the three fallback paths; the two
miss-path tests fail against the previous code.
## Verification
- `pnpm vitest run src/lib/sw-offline-fallback.test.ts` in `ui/` — 3
tests pass.
- Verified both miss-path tests fail against the unmodified `sw.js`.
## Risks
Low risk. The change only affects the fetch-rejection path; successful
fetches and cache hits behave exactly as before. `Response.error()`
mirrors what the browser would produce for an unhandled failed no-cors
fetch.
## Model Used
- Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code
CLI with tool use (code search, edit, 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 searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
|
||
|
Devin Foley
|
2c53437fc9 |
fix(server): authenticate cloud-proxied browsers on the live-events websocket (#11290)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The UI receives live run/issue events over a websocket at `/api/companies/:id/events/ws`; the server authorizes upgrades with a bearer token or a Better Auth session > - On a cloud-managed deployment, browsers authenticate through trusted `x-paperclip-cloud-*` headers injected by the managing front door — they never hold a local Better Auth session, and the Express middleware lane that understands those headers is not consulted for websocket upgrades > - Every browser websocket upgrade behind the front door therefore resolves no identity and is rejected 403: the live-events socket has never connected on a managed instance, leaving permanent reconnect churn and console failure noise while the UI silently degrades to polling > - This pull request adds a cloud-actor lane to the upgrade authorization, reusing the same trusted-header resolver the HTTP middleware uses > - The benefit is working realtime updates on managed instances, an end to the reconnect churn, and unchanged self-hosted behavior ## Linked Issues or Issue Description No existing issue. Description follows the bug template: **What happened?** On a cloud-managed instance, the browser console shows `WebSocket connection to 'wss://…/api/companies/<id>/events/ws' failed:` repeating indefinitely for every company, on a healthy instance. The server rejects each upgrade with 403 because `authorizeUpgrade` in `server/src/realtime/live-events-ws.ts` only knows bearer tokens and Better Auth sessions, while cloud-proxied browsers authenticate via `x-paperclip-cloud-*` trusted headers (handled only by the Express `actorMiddleware` lane in `server/src/middleware/auth.ts`). **Expected behavior** A browser that authenticates through the trusted cloud headers can open the live-events websocket for any company in its membership scope, exactly as it can call the HTTP API for those companies. **Steps to reproduce** 1. Run Paperclip in `authenticated` mode behind a proxy that injects the `x-paperclip-cloud-*` headers with a valid `PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN`. 2. Load any company page in a browser (no local Better Auth session). 3. HTTP API calls succeed; every `/events/ws` upgrade is rejected 403 and the UI retries forever. ## What Changed - `server/src/middleware/auth.ts`: `resolveCloudTenantActor` now accepts a minimal `CloudActorHeaderSource` (`header(name)`) instead of an Express `Request` — `Request` satisfies it unchanged — plus `cloudActorHeaderSourceFromHeaders` to adapt raw `IncomingMessage.headers`. - `server/src/realtime/live-events-ws.ts`: `authorizeUpgrade` gains an injected `resolveCloudActor` lane, tried before the Better Auth session fallback in `authenticated` mode. A resolved cloud actor is authoritative: the upgrade is authorized only for a company in the actor's membership scope (`companyIds`, the same scope the HTTP lane grants). Absent/unresolvable cloud headers fall through to the session path. - `server/src/index.ts`: wires `resolveCloudActor` through `resolveCloudTenantActor` + the header shim. The resolver self-gates: without `PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN` and a matching trust token it returns null, so self-hosted deployments never take this path. - Tests: upgrade authorized for an in-scope company (session resolver not consulted), rejected for an out-of-scope company, fall-through to session auth when no cloud actor resolves; header-shim resolution from a raw lowercased header map including `string[]` values. ## Verification - `pnpm vitest run server/src/__tests__/live-events-ws.test.ts server/src/middleware/cloud-tenant-actor.test.ts` — 25 tests pass. - `pnpm typecheck` in `server/` — clean. - Not verified live end-to-end: that requires a managed instance running this build; the direct probe evidence (HTTP authenticated fine, every WS upgrade 403) matches the code path exactly. ## Risks Low risk. The new lane only activates when the deployment configures the cloud trust token and the request presents it; both checks already protect the HTTP lane. Authorization scope is the same `companyIds` set the HTTP middleware computes (primary stack company plus the user's real membership rows). The cloud resolver's user/company materialization writes are debounced (existing behavior shared with the HTTP lane), so websocket reconnect storms do not amplify database writes. Self-hosted instances see no behavioral change, covered by the fall-through test. ## Model Used - Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code CLI with extended thinking and tool use (code search, edit, test execution; diagnosis included live websocket handshake probes against a managed instance). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting mergecanary/v2026.812.0-canary.9 |
||
|
Nicky LeachandPaperclip
|
6a5b293240 |
test(server): fix onboarding first-task teardown foreign-key race (#11284)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server coordinates issue work and heartbeat runs. > - The onboarding first-task route sends an assignment wake in the background. > - The route test removes related database rows during teardown. > - A late heartbeat run can keep foreign-key child rows alive during teardown. > - This pull request drains the wake and deletes run rows in foreign-key order. > - The benefit is a stable test that keeps the onboarding behavior unchanged. ## Linked Issues or Issue Description **What happened?** The onboarding first-task route sent a background assignment wake. The test teardown removed parent rows before the wake-created heartbeat rows finished. **Expected behavior** The test teardown should wait for the background wake and remove heartbeat rows before it removes their parent rows. **Steps to reproduce** 1. Run the onboarding first-task route test. 2. Repeat the test many times. 3. Observe an intermittent foreign-key error during teardown. **Paperclip version or commit** Commit `c30fe965920eeb7e7fb88e17574a65bed8fc01a4`. **Deployment mode** Local dev (pnpm dev). **Installation method** Built from source (pnpm dev / pnpm build). **Agent adapter(s) involved** Not adapter-specific (core bug). **Database mode** Embedded PGlite (default — DATABASE_URL unset). ## What Changed - Stub the server adapter in the route test so the dispatched run finishes at once. - Drain heartbeat runs to quiescence before teardown. - Delete heartbeat runs and child rows before their parent rows. - Delete runtime state and company skill rows in foreign-key order. - Keep the route behavior and all three test assertions unchanged. ## Verification - Run `pnpm exec vitest run src/__tests__/issue-onboarding-first-task-routes.test.ts` from the `server` package. - The author ran the suite 25 times with 25 passes. - The suite reproduced the teardown foreign-key error before this change. ## Risks Low risk. This change affects one test file and does not change product code or route behavior. ## Model Used OpenAI GPT-5. The model used tool calls and code review assistance. The exact context window and reasoning mode were not exposed in this run. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>canary/v2026.812.0-canary.8 |
||
|
Nicky LeachandPaperclip
|
f9bd0438e1 |
fix(server): stop terminal workspace reaper starving on oldest candidates (#11238)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server runs a scheduled reaper that archives terminal workspaces after it checks their state. > - The reaper reads candidates in `updatedAt` order and skips candidates that do not qualify for archive. > - The fixed page kept the same skipped candidates at the front, so the reaper did not inspect later eligible workspaces. > - This pull request adds a keyset cursor and a throttled log for sweeps that archive no workspace. > - The benefit is that the reaper inspects all candidates over time and reports an inert sweep. ## Linked Issues or Issue Description **What happened?** The terminal workspace reaper inspected a fixed page of old candidates. Ineligible candidates stayed in that page, so the reaper skipped later eligible workspaces on every sweep. **Expected behavior** The reaper must inspect each candidate over time and archive every eligible terminal workspace. **Steps to reproduce** 1. Create more than 50 terminal workspace candidates. 2. Keep the oldest page ineligible for archive. 3. Place an eligible workspace after that page. 4. Run repeated reaper sweeps. 5. Observe that the later eligible workspace remains unarchived. **Paperclip version or commit** Commit `3efdf555e6e14a46747c796c3c554438bfc03261`. **Deployment mode** Built from source with the server test suite. **Installation method** Built from source. **Agent adapter(s) involved** Not adapter-specific (core bug). **Database mode** Not database-related. ## What Changed - Add a keyset cursor that uses `(updatedAt, id)` order across reaper pages. - Reset the cursor at the end of the candidate set so the next sweep starts at the beginning. - Add a throttled log when a sweep inspects candidates but archives none. - Add regression tests for archive delivery and starvation. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/execution-workspaces-service.test.ts` — 45 tests pass. - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/server-startup-feedback-export.test.ts` — 16 tests pass. - `pnpm --filter @paperclipai/server typecheck` — clean. ## Risks Low risk. The change affects only candidate paging and the related reaper log. The cursor resets after the candidate set, so the sweep remains periodic. ## Model Used Codex, OpenAI GPT-5, extended reasoning, tool use, and code 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 searched GitHub for duplicate or related PRs and linked them above (no exact duplicate found; related scheduler PR #10911 is distinct) - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (no documentation applies; this is an internal reaper behavior change) - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>canary/v2026.812.0-canary.7 |
||
|
DottaandPaperclip
|
1a377424db |
fix(ui): add mobile blocker actions (#11282)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operators use task properties to inspect and change task relationships. > - A blocked-by chip linked directly to the blocking task. > - Its remove control appeared only on hover, so touch users could not reach it. > - This pull request opens a small action menu when a user taps the chip on mobile. > - The menu lets the user visit the task or start the existing blocker-removal confirmation. > - The benefit is that touch users can manage blockers without changing the fast desktop flow. ## Linked Issues or Issue Description **What happened?** On a phone-width layout, a tap on a blocked-by chip opened the blocking task immediately. The remove control appeared only on hover, so a touch user could not remove the blocker. **Expected behavior** A tap on a blocked-by chip on mobile opens a menu. The menu offers `Visit task` and `Remove blocker` actions. **Steps to reproduce** 1. Open a task that has a blocker. 2. Use a viewport below the mobile breakpoint. 3. Open the task properties. 4. Tap the blocked-by chip. **Paperclip version or commit** Reproduced on `e5a7fd7038` from `master`. **Deployment mode** Built from source with the local development workflow. ## What Changed - Added a mobile-only action menu to blocked-by chips. - Kept the direct task link and hover/focus remove control on desktop. - Reused the existing removal confirmation before the relation update. - Added focused regression coverage for the mobile visit and remove choices. - Added a phone-width Storybook state with the action menu open. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/components/IssueProperties.test.tsx` - `pnpm --filter @paperclipai/ui typecheck` - `pnpm check:token-gates` - `pnpm build-storybook` - Opened the new Storybook state in Playwright Chromium with a Pixel 5 viewport. Confirmed that both actions are visible and fit in the viewport. ## Risks - Low risk. The behavior change is limited to the existing mobile breakpoint. - Desktop navigation and blocker removal keep their current behavior. - The menu uses the shared dropdown and dialog primitives. > 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 `gpt-5.6-sol`, xhigh reasoning. The Codex CLI managed the context window for this run. The model used repository tools, code execution, tests, and browser automation. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>canary/v2026.812.0-canary.6 |
||
|
Nicky LeachandPaperclip
|
e5a7fd7038 |
Add sandbox device-login for the Codex adapter (#11237)
## Thinking Path > - Paperclip helps people manage AI agents for work. > - Agent adapters connect Paperclip to tools such as the Codex command line tool. > - A sandboxed Codex agent may start without a credential. > - The operator needs a safe sign-in flow that does not expose credentials to the shared package or the sandbox. > - This pull request adds a company-scoped device-login flow with a temporary Daytona sandbox. > - The flow promotes the credential only after readiness checks pass and removes the temporary sandbox after use. > - The result lets an operator sign in to a sandboxed Codex agent from the agent form. ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting (multiple of the above) **Problem or motivation** A Codex adapter that runs in a sandbox cannot authenticate when the company has no pre-provisioned Codex credential. **Proposed solution** Add a company-scoped device-login session. Start a temporary sandbox, run `codex login --device-auth`, stream the code and URL, verify readiness, promote the credential, and delete the sandbox. **Alternatives considered** Pre-provisioning a credential does not support first-time sandbox login. Keeping the credential in the login sandbox does not provide a durable company credential. **Roadmap alignment** This supports the roadmap item for cloud and sandbox agents. **Additional context** The flow uses a five-minute cleanup reaper, compare-and-set status changes, and a PostgreSQL advisory lock to protect promotion and cleanup. ## What Changed - Add the adapter login-session contract, database table, and migration. - Add company-scoped server routes and a service for sandbox device login. - Add credential promotion, readiness checks, and cleanup after login. - Add restart-safe cleanup for abandoned login sandboxes. - Add sandbox login controls to the agent creation and edit forms. - Keep device-login and vendor identifiers out of public shared and adapter UI symbols. ## Verification - `pnpm --filter @paperclipai/adapter-codex-local exec vitest run` passed with 310 tests at the submitted commit. - The server login route, service, and reaper tests passed with 45 tests at the submitted commit. - The agent form render tests passed with 26 tests at the submitted commit. - The public-symbol leak check passed at the submitted commit. - A live Daytona sign-in flow still requires confirmation by a user with a live sandbox. ## Risks The migration adds a new company-scoped table. A promotion or cleanup race could remove a credential or leave a sandbox active, so the service uses claims, compare-and-set transitions, and an advisory lock. The live Daytona flow needs operator confirmation because local tests do not provide a real browser sign-in. ## Model Used OpenAI Codex, GPT-5, tool use and code execution, extended reasoning. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>canary/v2026.812.0-canary.5 |
||
|
DottaandPaperclip
|
9c941169a6 |
fix(ui): keep new task dialog visible above mobile keyboard (#11281)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operators create tasks in a dialog that includes the assignee and project fields. > - Mobile browsers reduce and offset the visual viewport when the on-screen keyboard opens. > - The dialog used layout viewport units, so its upper fields could move off-screen while the user typed. > - This pull request makes the dialog follow the live visual viewport and keeps the focused editor visible. > - The benefit is that operators can see the task context and the field they edit on mobile devices. ## Linked Issues or Issue Description **What happened?** On mobile browsers, opening the keyboard in the new-task dialog could move the assignee and project fields above the visible screen. The active editor could also become difficult to see. **Expected behavior** The full dialog must stay inside the visible browser area. The active editor and task controls must remain reachable while the on-screen keyboard is open. **Steps to reproduce** 1. Open Paperclip on a mobile browser. 2. Open the new-task dialog. 3. Focus the title or description editor to open the on-screen keyboard. 4. Observe that the upper fields can move outside the visible viewport. **Paperclip version or commit** Reproduced before commit `838cdbb325` on `master`. **Deployment mode** Local dev (`pnpm dev`) in a mobile browser viewport. ## What Changed - Read `window.visualViewport` while the dialog is open. - Apply token-based dialog geometry when the visual viewport is constrained. - Keep the focused editor visible after viewport resize and scroll events. - Add unit coverage for visual viewport updates and focus scrolling. - Add Playwright coverage for mobile, tablet, desktop keyboard, and unconstrained desktop layouts. ## Verification - `pnpm exec vitest run ui/src/components/NewIssueDialog.test.tsx` — 27 tests passed. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm check:token-gates` — passed with all gates clean. - `pnpm --filter @paperclipai/ui build-storybook` — passed. - `pnpm exec playwright test tests/storybook-visual/new-issue-dialog-viewport.spec.ts --config tests/storybook-visual/playwright.config.ts` — 4 tests passed. ## Risks - Low risk. The custom geometry only activates when `visualViewport.height` is less than `window.innerHeight`. - Browsers without the Visual Viewport API keep the existing dialog primitive behavior. - The browser test checks hit targets and visible bounds at mobile, tablet, and desktop widths. > 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, GPT-5. The session used reasoning, repository tools, shell execution, and browser automation. The service did not expose the context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>canary/v2026.812.0-canary.4 |
||
|
Nicky LeachandClaude Opus 4.8
|
67001ec6eb |
chore(db): treat Drizzle migration snapshots as binary in diffs (#11254)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Paperclip stores its state in a Postgres database, managed in `packages/db`. > - The schema uses Drizzle. `drizzle-kit` writes a full-schema snapshot to `packages/db/src/migrations/meta/` for every migration. > - Each snapshot is a large generated JSON file. One snapshot is about 39k lines. > - PR #11240 marked these files `linguist-generated=true`. That collapses the file view and drops the files from language stats. > - But `linguist-generated` does not change the PR line-count badge. Git still counts every snapshot line, so a PR that adds one migration shows a ~39k-line badge (two migrations show ~78k). PR #11237 shows +84,665 for this reason. > - This pull request adds `-diff` to the same files, so git treats them as binary and their lines leave the +/- count. > - The benefit is a pull request badge that reflects the real code change, not generated snapshot noise. ## Linked Issues or Issue Description No existing issue. This is a small follow-up to merged PR #11240. Description follows the enhancement template: **Problem / Motivation** PR #11240 marked `packages/db/src/migrations/meta/**` as `linguist-generated=true`. That attribute collapses the diff in the Files-changed view and removes the files from language stats, but it does not remove their lines from the PR additions/deletions badge. Each Drizzle snapshot is a full copy of the schema (~39k lines), so any PR that adds a migration still shows a huge line count. PR #11237 shows +84,665, of which ~78k are two generated snapshots. **Proposed Solution** Add `-diff` to the same glob. Git then treats the snapshots as binary. `git diff --numstat` reports `-` for these files, so their lines leave the +/- badge and GitHub shows "Binary file not shown" in place of the full JSON. **Alternatives Considered** - Keep only `linguist-generated`: leaves the misleading ~39k/78k badge on every migration PR. - Use the `binary` macro (`-diff -merge -text`): also disables EOL normalization. This repo has open CRLF/LF work, so `-text` is left unset on purpose. ## What Changed - Added `-diff` to `src/migrations/meta/**` in `packages/db/.gitattributes`. - Kept `linguist-generated=true` (language stats) and `-merge` (no auto-merge of generated snapshots). - Left `-text` unset on purpose, so end-of-line normalization stays intact. - Left the `.sql` migration files untouched, so their diffs stay visible for review. ## Verification Check the attributes and confirm the snapshot is now treated as binary: ``` git check-attr linguist-generated diff merge -- \ packages/db/src/migrations/meta/0031_snapshot.json \ packages/db/src/migrations/0009_fast_jackal.sql git diff --numstat origin/master...origin/feat/adapter-sandbox-login -- \ packages/db/src/migrations/meta/0214_snapshot.json ``` Expected: ``` packages/db/src/migrations/meta/0031_snapshot.json: linguist-generated: true packages/db/src/migrations/meta/0031_snapshot.json: diff: unset packages/db/src/migrations/meta/0031_snapshot.json: merge: unset packages/db/src/migrations/0009_fast_jackal.sql: linguist-generated: unspecified packages/db/src/migrations/0009_fast_jackal.sql: diff: unspecified packages/db/src/migrations/0009_fast_jackal.sql: merge: unspecified - - packages/db/src/migrations/meta/0214_snapshot.json ``` The snapshot reports `-` in numstat (binary, not counted). The `.sql` migration keeps normal diff behavior. GitHub reads the rule from the PR tree, so the badge drops on the next PR that touches these files. ## Risks Low risk. The change only affects how git and GitHub render and count generated files. It does not touch application code, the schema, or any migration. - With `-diff`, GitHub and local `git diff` no longer show a text diff for a snapshot. This is intended; the files are generated and are not reviewed by hand. The raw file is still viewable. - `-text` is left unset, so this change does not affect the CRLF/LF handling that other PRs (for example #8922) address. ## Model Used Claude Opus 4.8 (Anthropic), model ID `claude-opus-4-8`, used through Claude Code with extended thinking and tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass — not applicable; this change touches no code path, only git/GitHub file handling - [x] I have added or updated tests where applicable — not applicable; `.gitattributes` behavior is verified with `git check-attr` and `git diff --numstat` (see Verification) - [x] I have updated relevant documentation to reflect my changes — not applicable; the `.gitattributes` file documents its own rules inline - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>canary/v2026.812.0-canary.3 |
||
|
Devin Foley
|
f16071a290 |
fix(ui): use a fictional tailnet hostname in the vite proxy test (#11248)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The UI dev server proxies `/api` to the backend and injects `x-forwarded-host`; a unit test asserts that injection with a sample Host header > - The sample Host header is a contributor's real machine and tailnet hostname, committed to the public repository > - Real personal hostnames do not belong in a public codebase, and this one also contains the contributor's OS username, so `scripts/check-forbidden-tokens.mjs` (which forbids the local username) blocks `npm` publishing from that contributor's machine > - This pull request replaces the fixture with a fictional tailnet-style hostname > - The benefit is no personal identifiers in the test fixtures and a passing forbidden-token check for every contributor ## Linked Issues or Issue Description No existing issue. Description follows the bug template: **What happened?** `ui/src/lib/vite-api-proxy.test.ts` uses a real contributor dev-machine hostname as its `Host` header fixture. `node scripts/check-forbidden-tokens.mjs` fails on that contributor's machine because the hostname contains their OS username, blocking the publish flow. Introduced in #10718. **Expected behavior** Test fixtures use fictional hostnames. The forbidden-token check passes on every contributor machine. **Steps to reproduce** 1. On a machine whose OS username appears in the fixture hostname, run `node scripts/check-forbidden-tokens.mjs`. 2. The check reports the two lines in `ui/src/lib/vite-api-proxy.test.ts` and blocks with exit code 1. ## What Changed - `ui/src/lib/vite-api-proxy.test.ts`: the `Host` fixture is now `dev-box.tail1234.ts.net:3101` (fictional). The test only asserts that whatever host arrives is injected as `x-forwarded-host`, so the value is arbitrary. ## Verification - `pnpm vitest run src/lib/vite-api-proxy.test.ts` in `ui/` — 5 tests pass. - `node scripts/check-forbidden-tokens.mjs` — "No forbidden tokens found" on the previously affected machine. - Note: git history retains the old value; this removes it from the current tree only. ## Risks None. A test fixture string with no behavioral coupling. ## Model Used - Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code CLI with tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting mergecanary/v2026.812.0-canary.2 |
||
|
Devin Foley
|
d90f4d488e |
fix(ui): survive first load against a cold backend without a blank page (#11246)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The web UI holds live websocket connections for run events, coordinates cross-tab polling through a leader-election store, and renders app chrome (sidebar, providers) around a routed outlet > - When the backend is still cold-starting (managed hosting wake, server restart, reverse proxy up before the app), the event websockets refuse connections and the first SPA load mounts against a dead backend > - In that state the mount cascade can exceed React's nested update limit (minified error #185); the crash originates in shell hooks outside the routed error boundary, so React unmounts the entire root to a blank page, and the dead page keeps retrying the websocket on a flat 1.5s timer until the user hard-refreshes > - This pull request removes the wasted nested commits from the shared-polling subscription path, adds exponential backoff to the transcript websocket reconnect, and adds a last-resort app-shell error boundary > - The benefit is that a cold or briefly unreachable backend degrades to a recoverable state instead of a blank page that hammers the server ## Linked Issues or Issue Description No existing issue. Description follows the bug template: **What happened?** On the first load against a backend that was still starting, the app showed its loading animation and then a blank page. The console showed repeated `WebSocket connection to 'wss://…/api/companies/<id>/events/ws' failed` lines and `Uncaught Error: Minified React error #185` with a stack through the shared-polling coordinator's `subscribe`. The websocket retries continued indefinitely on the dead page. A manual refresh fixed it. **Expected behavior** A backend that is briefly unreachable degrades gracefully: websocket reconnects back off, the UI keeps rendering from cache, and even a worst-case crash shows a reload prompt instead of a blank page. **Steps to reproduce** 1. Serve the UI while the backend API is still starting (websocket upgrades and API calls refused). 2. Load any company page with several shared-polling consumers mounted (dashboard with sidebar). 3. Observe repeated websocket failures; on affected loads the page goes blank with React error #185. ## What Changed - `ui/src/hooks/useSharedPolling.ts`: coordinator snapshot notifications now keep the previous state object when leadership did not change, so React bails out instead of scheduling a nested re-render. `subscribe` invokes its listener synchronously from inside the mount effect with a fresh object each time; before this change every mount and notify burned nested-update budget even with no value change — the crash frame in the field report was exactly this `subscribe → setState` call. - `ui/src/components/transcript/useLiveRunTranscripts.ts`: the live event websocket reconnect backs off exponentially (1.5s → 15s cap, reset on successful open), mirroring `LiveUpdatesProvider`, instead of a flat 1.5s retry. - `ui/src/components/AppErrorBoundary.tsx` (+ wiring in `ui/src/main.tsx`): a dependency-free boundary above the router and providers. `RouteErrorBoundary` only guards the routed `<Outlet />`; a crash in the shell around it had no boundary, so React unmounted the root to a blank page. The boundary renders a reload prompt with the error message. - Tests: `useSharedPollingSnapshot.test.tsx` (mount costs no extra commit — fails against the previous code; a real leadership change re-renders exactly once and ticks stay quiet), a backoff test in `useLiveRunTranscripts.test.tsx` (delays grow 1.5s → 3s → 6s and reset after a successful open), and `AppErrorBoundary.test.tsx` (render throw, effect throw, healthy pass-through). ## Verification - `pnpm vitest run` in `ui/` over the touched suites (shared polling, cross-tab poll, transcripts, boundary): 34 tests pass. - `pnpm typecheck` in `ui/` — clean. - The snapshot regression test was verified to fail against the pre-change hook (extra commit per mount). - Not reproduced end-to-end: the exact 50-update cascade from the field crash needs a live cold backend; the change removes the identified per-mount/per-notify nested commits at the reported crash frame, bounds the reconnect load, and guarantees the shell can no longer blank the page. ## Risks Low risk. The snapshot change only suppresses re-renders whose state is value-identical; leadership changes propagate exactly as before. The backoff only lengthens retry delays after consecutive failures and resets on success. The new boundary renders children untouched unless an error reaches it; behavior on healthy loads is unchanged. Self-hosted deployments see the same code paths — the cold-backend window simply rarely occurs there. ## Model Used - Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code CLI with extended thinking and tool use (code search, edit, test execution; diagnosis included mapping the production minified stack to source). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Devin Foley
|
9adeb4a9d0 |
fix(ui): fetch the web app manifest with credentials (#11245)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The web UI is a PWA-capable SPA; `ui/index.html` links `/site.webmanifest` so browsers can read app metadata > - Browsers fetch `<link rel="manifest">` in "omit credentials" mode unless the link opts in with `crossorigin="use-credentials"` > - Self-hosted this is harmless, but when Paperclip runs behind an authenticating reverse proxy (a managed hosting front door), the cookie-less manifest request is rejected with 401 on every page load and logs a console error pair on each navigation > - This pull request adds `crossorigin="use-credentials"` to the manifest link so the request carries the same session cookies as every other same-origin asset request > - The benefit is a clean console and a servable manifest in proxied deployments, with self-hosted behavior unchanged ## Linked Issues or Issue Description No existing issue. Description follows the bug template: **What happened?** On every page load behind an authenticating reverse proxy, the browser logs `Failed to load resource: the server responded with a status of 401` for `/site.webmanifest`, plus `Manifest fetch from … failed, code 401`. The proxy rejects the request because the browser sends the manifest fetch without cookies. **Expected behavior** The manifest request carries the same session credentials as every other same-origin asset request, so the proxy can authenticate and serve it. No console errors. **Steps to reproduce** 1. Serve Paperclip behind a reverse proxy that requires a session cookie for all app routes. 2. Sign in and load any page. 3. Open the browser console: the manifest fetch fails with 401 while all other assets load. ## What Changed - `ui/index.html`: the manifest link now carries `crossorigin="use-credentials"`. - `ui/src/lib/pwa-install-mode.test.ts`: a regression test asserts the attribute stays on the link. ## Verification - `pnpm vitest run src/lib/pwa-install-mode.test.ts` in `ui/` — 2 tests pass. - Manual check of the rendered link tag in `ui/index.html`. ## Risks Low risk. The manifest is same-origin, so `use-credentials` only switches the fetch from "omit" to the include behavior all other same-origin requests already have. Self-hosted deployments see no change. Cross-origin manifest hosting is not used in this project. ## Model Used - Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code CLI with extended thinking and tool use (code search, edit, 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 searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Devin Foley
|
d5bb396518 |
fix: pass sandbox provider credential env vars to plugin workers; hide Local default under managed-sandbox-only (#11244)
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Environments give each agent run an execution target, and sandbox providers (Daytona, E2B, Novita, exe.dev) run as plugin workers > - A managed deployment provisions one platform-managed sandbox row with no credential in config; the provider is documented to fall back to its process env var (for example `DAYTONA_API_KEY`) > - Plugin workers spawn with a scrubbed environment, so that fallback never sees the host env var — probe and lease acquisition fail with "require an API key in config or DAYTONA_API_KEY" even when the deployment sets the var > - Separately, the managed-sandbox-only mode hides local rows from every list, but the instance Default picker renders a hardcoded synthetic "Local" option that no filter touches > - This pull request forwards each bundled provider's documented credential env var to its own plugin worker, and gates the synthetic Local option on the flag > - The benefit is that the documented host-env credential fallback works for plugin-backed providers, and managed-sandbox-only instances no longer offer Local anywhere ## Linked Issues or Issue Description **Subsystem affected** Plugin worker environment construction (`server/src/services/plugin-loader.ts`) and the environments UI (instance Default picker, agent form inherited-environment label). **Problem or motivation** Two follow-ups to the managed-sandbox-only mode (#11200), both found on a live managed deployment: 1. The deployment sets `DAYTONA_API_KEY` as a server env var and the managed sandbox row omits `config.apiKey` by contract. "Test Connection" fails with `Sandbox environment probe failed for provider "daytona". Daytona sandbox environments require an API key in config or DAYTONA_API_KEY.` A real agent run fails the same way at lease acquisition. The cause: sandbox providers run as plugin workers, and `buildPluginWorkerEnv` passes only model-provider keys and in-cluster Kubernetes vars. The provider's own documented credential env var never reaches the worker, so the in-plugin `process.env` fallback reads nothing. The self-hosted path has the same gap: the Daytona plugin README documents `DAYTONA_API_KEY` as a host-level fallback, and it does not work today. 2. With `enableManagedSandboxOnly` on, the instance Default environment picker still shows "Local". The server filters local *rows* out of the list, and the client filter mirrors that for cached lists, but this option is a hardcoded `<option value="">Local</option>` — not a list row — so no filter removes it. Selecting it writes a null default, which run selection then rejects fail-closed. **Proposed solution** Forward each bundled sandbox provider's documented credential env var into its plugin worker, keyed by the manifest's declared `environmentDrivers[].driverKey` so a worker only receives its own provider's credential (daytona → `DAYTONA_API_KEY`, e2b → `E2B_API_KEY`, exe-dev → `EXE_API_KEY`, novita → `NOVITA_API_KEY`). Keep the existing gate: only plugins that declare `environment.drivers.register` receive any passthrough. In the UI, render the synthetic Local option only when managed-sandbox-only is off; under the flag show a disabled "Select environment" placeholder only while no default is stamped yet, and stop the agent form's inherited label from reading "Local". **Alternatives considered** Adding `DAYTONA_API_KEY` to the existing `ADAPTER_ENV_PASSTHROUGH` list was rejected: that list goes to every environment-driver plugin, so each provider would receive every other provider's credential. A manifest schema field for declared credential env vars was rejected as heavier than needed: the bundled providers are known, and the mapping lives next to the two existing passthrough lists. ## What Changed - `server/src/services/plugin-loader.ts`: new `SANDBOX_PROVIDER_CREDENTIAL_ENV_PASSTHROUGH` map (driverKey → documented credential env vars). `buildPluginWorkerEnv` reads the manifest's `environmentDrivers` and forwards only the matching vars, after the existing `environment.drivers.register` gate. Blank values stay excluded. - `server/src/__tests__/plugin-database.test.ts`: the daytona worker receives `DAYTONA_API_KEY` and not another provider's key; a plugin whose drivers have no mapping (kubernetes) receives no credential var. - `ui/src/pages/CompanyEnvironments.tsx`: the Default picker's synthetic Local option renders only when managed-sandbox-only is off. Under the flag, a disabled "Select environment" placeholder renders only while the default is unset. - `ui/src/pages/CompanyEnvironments.test.tsx`: the Local option is present by default and absent under the flag; saved non-local environments stay selectable. - `ui/src/components/AgentConfigForm.tsx`: the inherited-environment label falls back to "Managed sandbox" instead of "Local" under the flag. ## Verification - `server`: `npx vitest run src/__tests__/plugin-database.test.ts -t buildPluginWorkerEnv` — 5 passed (3 existing, 2 new). - `ui`: `npx vitest run src/pages/CompanyEnvironments.test.tsx` — 22 passed (2 new); `npx vitest run src/components/AgentConfigForm.render.test.tsx` — 10 passed. - `tsc --noEmit` clean in `server` and `ui`. - Live managed deployment: confirmed the tenant service env carries `DAYTONA_API_KEY` while the probe fails with the exact message above, which pins the root cause to the worker env, not delivery. ## Risks - The worker env grows by exactly one var per matching bundled provider, only when the deployment sets it and only for plugins that declare a matching environment driver. Plugins without a mapping see no change. - Self-hosted behavioral shift is the fix itself: a host-level `DAYTONA_API_KEY` (or E2B/EXE/NOVITA equivalent) now reaches the provider as its README documents. Deployments that set the var but expected it to stay inert had no working configuration to preserve — the provider errored on every keyless probe and run. - UI change is inert unless `enableManagedSandboxOnly` is on (default false everywhere). ## Model Used Claude Fable 5 (`claude-fable-5`) via Claude Code — extended thinking, tool use, parallel read-only subagents for the two root-cause traces. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting mergecanary/v2026.812.0-canary.1 |
||
|
Devin Foley
|
0aa743fc30 |
build(db): clean dist before drizzle generate (#11241)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Database migrations are generated by drizzle-kit, which reads the schema from the db package's built `dist/schema/*.js` > - `tsc` never deletes stale outputs, so a long-lived checkout keeps compiled schema files whose sources were deleted long ago > - A `generate` run in such a checkout sees those ghost tables and sweeps phantom `CREATE TABLE` statements into an unrelated migration > - This pull request makes `generate` clean `dist` before building, so drizzle always diffs against exactly the current schema sources > - The benefit is that no contributor can accidentally resurrect deleted tables inside a new migration ## Linked Issues or Issue Description **What happened?** Running `pnpm --filter @paperclipai/db generate` in a months-old checkout produced a migration re-creating `cloud_upstream_connections`, `cloud_upstream_runs`, and `company_secret_pools` — tables whose schema sources were deleted in #10507. The compiled copies were still in `dist/schema/`, and `drizzle.config.ts` reads the schema from `dist`, so drizzle treated them as new tables missing from the snapshot. **Expected behavior** `generate` diffs the current schema sources only; deleted tables can never reappear in a generated migration. **Steps to reproduce** 1. Build the db package, then delete a schema source file without cleaning `dist`. 2. Run `pnpm --filter @paperclipai/db generate`. 3. The generated migration re-creates the deleted table. ## What Changed - `packages/db/package.json`: the `generate` script runs `pnpm run clean` before `tsc`, so the drizzle-kit input is always a fresh build of the current sources. ## Verification - In a checkout carrying stale `dist/schema/cloud_upstreams.js` / `company_secret_pools.js` artifacts, `generate` produced a phantom migration before this change; after a clean build it reports "No schema changes, nothing to migrate". With this change the clean happens inside `generate` itself. ## Risks - Low risk: `generate` is a developer-only script; the change only adds the existing `clean` step ahead of the existing build, at the cost of a full rebuild per generate. ## Model Used - Claude Fable 5 (`claude-fable-5`) via Claude Code CLI, with extended thinking and tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting mergecanary/v2026.812.0-canary.0 |
||
|
Valentin Marchaud
|
b5ebda1dca |
fix(grok-local): report real token usage and cost instead of hardcoded zeros (#10433)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Cost/usage tracking is core to that: the dashboard shows per-agent spend so a team can see what their AI workforce is costing them > - The `grok_local` adapter (xAI's Grok Build CLI) is a newer adapter than `claude_local`/`codex_local`, and its usage/cost wiring was left incomplete > - Every `grok_local` run persists `usage.inputTokens/outputTokens/cachedInputTokens = 0` and `costUsd = null` in `heartbeat_runs`, unconditionally, even though the underlying `grok` CLI reports real, non-zero token counts and cost per turn in its own JSON stream > - This pull request wires the parser to actually read `usage`/`total_cost_usd` from the CLI's terminal `end` event, threads those values into the adapter's execution result, and marks them `usageBasis: "per_run"` so the heartbeat service doesn't incorrectly delta them against a prior run on a resumed session (matching how `claude_local`/`codex_local` already do this) > - The benefit is accurate cost/usage visibility for any self-hosted Paperclip instance running Grok Build agents, instead of a dashboard that always reads zero ## Linked Issues or Issue Description Fixes: #10432 ## What Changed - `packages/adapters/grok-local/src/server/parse.ts`: `parseGrokJsonl()` now reads `usage.input_tokens` / `usage.output_tokens` / `usage.cache_read_input_tokens` / `total_cost_usd` from the terminal `end` event and returns them on `ParsedGrokJsonl` (previously discarded entirely). - `packages/adapters/grok-local/src/server/execute.ts`: `toResult()` now populates `usage.inputTokens/outputTokens/cachedInputTokens` from the parsed values instead of hardcoded `0`, sets `usageBasis: "per_run"` (each `--single` invocation reports usage for just that process, not a running session total), and surfaces `costUsd` only when `billingType === "api"` (metered) — subscription/OAuth billing has no marginal dollar cost, so it stays `null` there, but token counts are populated for both billing types since usage visibility is useful regardless of billing model. - `packages/adapters/grok-local/src/server/parse.test.ts`: added a test asserting usage/cost extraction from a representative `end` event payload, and updated the existing exact-equality test for the new fields. - `packages/adapters/grok-local/src/server/execute.test.ts`: added a test covering both subscription billing (tokens populated, `costUsd: null`) and API-key billing (tokens populated, real `costUsd`), and asserting `usageBasis: "per_run"` in both cases. ## Verification - `pnpm vitest run packages/adapters/grok-local/src/server/parse.test.ts packages/adapters/grok-local/src/server/execute.test.ts` — 9/9 passed - `tsc --noEmit` on the `grok-local` package — clean - Verified against a real self-hosted Paperclip instance running `grok` CLI `0.2.112` with SuperGrok subscription (OAuth) auth: confirmed the raw CLI stream reports real `usage`/`total_cost_usd` (e.g. `"usage":{"input_tokens":21560,...},"total_cost_usd":0.0564448`) that was previously discarded before ever reaching `heartbeat_runs.usage_json`, which always showed all-zero tokens regardless of real usage. ## Risks - Low risk, additive change scoped entirely to the `grok_local` adapter's usage/cost reporting path — no change to control flow, session handling, or process execution. - `usageBasis: "per_run"` mirrors the existing, already-tested pattern in `claude_local`/`codex_local` execute paths, so the heartbeat service's per-run vs. session-cumulative delta logic is exercised the same way. - `costUsd` is intentionally left `null` for subscription/OAuth billing (no behavior change there beyond now-populated token counts) to avoid implying a dollar cost that doesn't exist for flat-rate billing. ## Model Used Claude Sonnet 5 (`claude-sonnet-5`), via Claude Code, no extended thinking. Root cause was found by comparing real `grok` CLI JSON stream output (captured directly from a live invocation) against the persisted `heartbeat_runs.usage_json` row for the same run on a self-hosted instance, then reading `parse.ts`/`execute.ts` source to confirm the hardcoded zero values. ## 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 (none found) - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (`fix/grok-local-usage-cost-tracking`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes (no user-facing docs reference this internal usage-reporting behavior) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending at time of writing) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (addressed the one P1 raised — `usageBasis: "per_run"`) - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Nicky LeachandClaude Opus 4.8
|
c0bdf26633 |
chore(db): collapse Drizzle migration snapshot diffs and block auto-merge (#11240)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Paperclip stores its state in a Postgres database, managed in `packages/db`. > - The schema uses Drizzle. `drizzle-kit` writes a full-schema snapshot to `packages/db/src/migrations/meta/` for every migration. > - Each snapshot is a large generated JSON file. One snapshot is over 200 KB. > - GitHub shows these files as 30k+ line diffs in a pull request. The diffs add no review value, because a human never edits the files. > - The files also merge badly. `drizzle-kit` computes the `id`/`prevId` chain and the full schema state, so a line-level merge of two snapshots produces a file that no real `generate` run creates. > - This pull request adds a `.gitattributes` file that marks the snapshot directory as generated and blocks its auto-merge. > - The benefit is clean pull request diffs and a loud conflict that forces the correct fix when two branches add a migration. ## Linked Issues or Issue Description No existing issue. This is a small repository-hygiene change. Description follows the enhancement template: **Problem / Motivation** Every migration adds a full-schema snapshot JSON under `packages/db/src/migrations/meta/`. These files are large and generated. GitHub renders them as 30k+ line diffs in pull requests, which buries the real change (the `.sql` migration) in noise. The files also have no meaningful line-level merge: `drizzle-kit` computes each snapshot's `id`/`prevId` chain and full schema state. **Proposed Solution** Add `packages/db/.gitattributes`: - `linguist-generated=true` on `src/migrations/meta/**` — GitHub collapses the diff and drops the files from language stats. - `-merge` on the same glob — git refuses the line-level merge and raises a conflict instead of fabricating an invalid snapshot. **Alternatives Considered** - `-diff` / `binary`: hides the diff completely and blocks text merge, but also blocks any local `git diff` and gives a worse conflict experience. `linguist-generated` keeps the file expandable and text-based, so it is the lighter option. - Do nothing: leaves the noisy diffs and the risk of a silent bad merge. ## What Changed - Added `packages/db/.gitattributes`. - Marked `src/migrations/meta/**` as `linguist-generated=true` to collapse the snapshot and journal diffs on GitHub. - Set `-merge` on the same files so git raises a conflict instead of auto-merging generated snapshots. - Left the `.sql` migration files untouched, so their diffs stay visible for review. ## Verification Run `git check-attr` against the affected files and a control `.sql` file: ``` git check-attr linguist-generated merge -- \ packages/db/src/migrations/meta/0031_snapshot.json \ packages/db/src/migrations/meta/_journal.json \ packages/db/src/migrations/0009_fast_jackal.sql ``` Expected output: ``` packages/db/src/migrations/meta/0031_snapshot.json: linguist-generated: true packages/db/src/migrations/meta/0031_snapshot.json: merge: unset packages/db/src/migrations/meta/_journal.json: linguist-generated: true packages/db/src/migrations/meta/_journal.json: merge: unset packages/db/src/migrations/0009_fast_jackal.sql: linguist-generated: unspecified packages/db/src/migrations/0009_fast_jackal.sql: merge: unspecified ``` The snapshot and journal files carry both attributes. The `.sql` migration keeps its normal diff and merge behavior. GitHub applies the rule from the pull request tree, so the collapse shows on the next pull request that touches these files. ## Risks Low risk. The change only affects git and GitHub display and merge behavior for generated files. It does not touch application code, the schema, or any migration. - `-merge` leaves the current-branch version in the working tree on conflict and marks the file conflicted. It does not insert conflict markers into the JSON. The correct resolution stays "renumber the later migration and regenerate", then commit. - Related open pull requests #879 and #8922 also add `.gitattributes` rules for migration files, but for CRLF/LF hash mismatches on Windows. If either lands, a follow-up can merge the rules into one file. There is no functional overlap with this change. ## Model Used Claude Opus 4.8 (Anthropic), model ID `claude-opus-4-8`, used through Claude Code with extended thinking and tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass — not applicable; this change touches no code path, only git/GitHub file handling - [x] I have added or updated tests where applicable — not applicable; `.gitattributes` behavior is verified with `git check-attr` (see Verification) - [x] I have updated relevant documentation to reflect my changes — not applicable; the `.gitattributes` file documents its own rules inline - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Hardik DholariyaandClaude Sonnet 4.6
|
3b74ff4813 |
fix(ui): use issuePrefix instead of name-derived prefix in create dialog badges (#8550)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents at work > - Paperclip stores an `issuePrefix` on each company (e.g. "OPS") used for issue identifiers (`OPS-1`) and company-prefixed routes (`/OPS/dashboard`) > - The create-dialog badges in NewIssueDialog, NewProjectDialog, and NewGoalDialog were derived from the company display name using `company.name.slice(0, 3).toUpperCase()` — so "Acme Labs" showed "ACM" > - This is misleading because the badge visually represents the issue prefix, but actually shows an unrelated 3-letter slice of the display name > - When a company has `issuePrefix = "OPS"` but `name = "Acme Labs"`, the badge showed "ACM" while issues use "OPS-1" > - This pull request replaces `name.slice(0, 3).toUpperCase()` with `company.issuePrefix` in all three dialog badge components > - The benefit is that the badge now matches the actual prefix used for issues and routes, eliminating confusion ## Linked Issues or Issue Description Fixes: #8501 ## What Changed - `ui/src/components/NewIssueDialog.tsx` (line ~1339): Replaced `company.name.slice(0, 3).toUpperCase()` with `company.issuePrefix` in the selected-company header badge - `ui/src/components/NewIssueDialog.tsx`: Replaced `company.name.slice(0, 3).toUpperCase()` with `company.issuePrefix` in the company picker list badge - `ui/src/components/NewProjectDialog.tsx`: Replaced `selectedCompany.name.slice(0, 3).toUpperCase()` with `selectedCompany.issuePrefix` in the selected-company header badge - `ui/src/components/NewGoalDialog.tsx`: Replaced `selectedCompany.name.slice(0, 3).toUpperCase()` with `selectedCompany.issuePrefix` in the selected-company header badge ## Verification 1. Create or configure a company whose `issuePrefix` differs from the first 3 letters of its display name (e.g. name = "Acme Labs", issuePrefix = "OPS") 2. Open the New Task dialog — the selected-company header badge should show "OPS", not "ACM" 3. Open the company picker dropdown inside the New Task dialog — each company list badge should show the actual `issuePrefix` 4. Open the New Project dialog — the selected-company header badge should show "OPS" 5. Open the New Goal dialog — the selected-company header badge should show "OPS" 6. Verify that companies whose prefix matches the first 3 letters (e.g. name="Ops Team", prefix="OPS") still display correctly **Before/After:** - Before: Company "Acme Labs" with `issuePrefix = "OPS"` showed badge "ACM" - After: Same company shows badge "OPS" (Screenshots require running the UI locally against a test instance with the relevant company configuration.) ## Risks Low risk — this is a purely visual change to 3 React component badge labels. No API changes, no schema changes, no behavioral changes to issue creation or routing. The `issuePrefix` field is already loaded on the company objects used by these components. ## Model Used - **Provider:** OpenCode - **Model:** MiMo v2.5 Free - **Reasoning:** N/A (standard code generation) ## 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 searched GitHub for duplicate or related PRs and found none targeting the same badge code - [x] I have linked the existing issue with Fixes: #8501 - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change (`fix/issue-8501`) - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>canary/v2026.811.0-canary.18 |
||
|
Devin Foley
|
0a95ada1be |
feat(server): chunked import preview endpoint and resumable upload in the Import page and CLI (#11224)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The previous pull request added server-side chunked resumable import transfers; without clients, large imports still ride the single fragile upload > - The Import page and the CLI need to slice large packages, upload parts with retry and progress, resume after interruptions, and preview before applying > - Preview is the missing server piece: the browser flow is preview-then-import, so a completed spool must be previewable without re-uploading > - This pull request adds the transfer preview endpoint, switches the Import page to the chunked path for zips over 48 MB, and teaches the CLI the same for oversized local imports > - The benefit is that large imports get progress, per-part retry, and resume in both clients, while small imports keep the exact single-shot path they have today ## Linked Issues or Issue Description **What happened?** With only the server transfer routes in place, users still upload large company packages as one request from the Import page and the CLI: no progress indication, no retry below the whole file, and no resume after a dropped connection or refresh. The preview-then-import flow also cannot run against an uploaded transfer, forcing a second full upload. **Expected behavior** A large package uploads once as verified parts with visible progress; preview and import both run against the uploaded spool; an interrupted upload resumes with only the missing parts re-sent; packages at or below 48 MB behave exactly as before. **Steps to reproduce** 1. Select a 500 MB zip on the Import page over an unreliable connection. 2. Watch the single upload fail near the end and restart from zero, twice — once for preview, once for import. 3. Same story headless via the CLI. ## What Changed - Server: `POST /import/transfers/:id/preview` runs the existing preview logic against the completed spool (shared assembly + whole-file verification helper with apply); preview neither completes the run nor deletes the spool, so the subsequent apply reuses it. Missing parts respond with the missing list. - UI: zips over 48 MB take the chunked path in both preview and import — the file is sliced into 32 MB parts hashed with WebCrypto (single ArrayBuffer, no second copy), the transfer is created or resumed (the create response's missing-parts list drives what uploads), parts upload sequentially with three attempts each and visible progress, then transfer preview/apply replace the multipart calls. The existing preview pane, collision handling, adapter overrides, and async job polling are unchanged; ≤ 48 MB keeps the single-shot path. - CLI: oversized local `.zip` or folder imports zip/slice/hash with node crypto, upload with resume and per-part retry and progress lines, and use transfer preview/apply. Small packages keep the inline path byte-identical. - Failure honesty: adapters/API errors fail open to existing behavior; a part failing all attempts surfaces a durable error panel with resume intact. ## Verification - Server: preview-then-apply on one spool (run stays open, spool intact, then apply completes), preview with missing parts rejected — added to the transfer route suite (embedded Postgres). - UI suite: large file takes the chunked path (manifest shape, part uploads, progress, apply on a resumed transfer, single-shot endpoints never called), small file stays single-shot, part failure after three attempts surfaces the error panel without running preview, resume re-uploads only the missing part. - CLI: manifest slicing/hashing, threshold behavior for zip and folder sources, folder-zip round-trip through the real zip reader, upload resume/retry/exhaustion/already-completed, full-command chunked and small-zip inline flows. - Server, ui, cli typechecks clean. Exact counts in the PR checks. ## Risks - The 48 MB threshold only routes between two verified paths; behavior below it is untouched. - Chunked CLI imports use the board-scoped transfer routes, so oversized CLI imports need board credentials (agent tokens keep the agent-safe small-file path). No privilege change — board actors already had the generic routes — but the two size regimes differ semantically; called out for review. - A CLI dry-run over the threshold uploads parts before previewing; the spool persists (24 h sweep) and a later apply resumes without re-upload — inherent to preview-against-spool. Stacked on #11223 — merge that first; this PR then shows only the preview endpoint and client changes. ## Model Used - Claude Fable 5 (`claude-fable-5`) via Claude Code CLI, with extended thinking and tool use (multi-agent implementation with independent verification). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting mergecanary/v2026.811.0-canary.17 |
||
|
Nicky LeachandPaperclip
|
8f478242f1 |
fix(adapter-utils): close sandbox stdin file race with atomic write and fault-tolerant poller (#11235)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent adapters use execution targets to exchange input and output with sandbox processes > - The sandbox input path can expose partial files, and the poller can delete or stop on invalid input > - These timing windows can lose agent input without a clear error > - This pull request makes host writes atomic and makes the poller retry invalid files before it drops them > - The benefit is reliable sandbox input delivery with visible failure after bounded retries ## Linked Issues or Issue Description Closes #10874 ## What Changed - Decode host input into a temporary file, then rename it onto the final JSON path. - Apply the same atomic write pattern to the filesystem client. - Parse each input file before deletion. - Retry parse failures and drop a file after the bounded retry limit with an error event. - Add regression tests for empty, partial, and permanently malformed input files. ## Verification - `pnpm exec vitest run packages/adapter-utils/src/execution-target-stdin-race.test.ts` passes 5/5 tests. - Related sandbox callback, execution target, and sandbox execution suites pass 71/71 tests. - TypeScript checks pass for the changed files. - The regression suite fails on the old code and passes on this change. ## Risks - Low risk. The change affects sandbox input file handling and adds bounded retry behavior. - A permanently malformed file now creates an error event after the retry limit. > This bug fix does not add a core feature, so a roadmap change is not needed. ## Model Used Codex, OpenAI GPT-5, current agent runtime, large context window, tool use and code review support. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Devin Foley
|
23a1b025c2 |
feat(server): chunked resumable company import transfers (#11223)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Company import moves large packages into an instance, and since the upload cap rose to 1 GB, the transport is the weak point: one HTTP request, buffered fully in memory, with no resume > - A dropped connection at 90% of an 800 MB upload starts the whole transfer over, and a server restart loses all progress > - This pull request adds the server side of chunked resumable import transfers: a durable run ledger and routes that accept the same import zip as verified ~32 MB parts spooled to disk > - An interrupted transfer resumes from the parts already uploaded — across dropped connections, page refreshes, and server restarts — and peak upload memory drops from the whole package to one part > - The benefit is that large imports become reliable on real-world connections instead of all-or-nothing ## Linked Issues or Issue Description **What happened?** Large company imports travel as a single HTTP upload. On a slow or flaky connection, any interruption discards all progress and the upload restarts from zero. The server buffers the entire compressed package in memory during upload. A server restart mid-upload loses the transfer entirely. With the upload cap now at 1 GB, these failure modes govern exactly the imports the cap was raised for. **Expected behavior** A large import upload survives interruptions: already-transferred data is kept and verified, only the missing remainder is re-sent, and the server's memory use during upload is bounded by a part, not the package. **Steps to reproduce** 1. Import a multi-hundred-MB company package over a connection that drops mid-upload. 2. The upload fails; retrying starts from byte zero. 3. Repeat on an unstable connection and the import may never complete. ## What Changed - New `company_transfer_runs` table (drizzle schema + migration) and `companyTransferRunService`: one row per transfer with a content-derived idempotency key, per-part completion recorded atomically and idempotently, resume scoped to actor and direction, completed runs short-circuiting retries of identical content. - New transfer routes beside the existing import routes, same authorization: declare a sliced zip (`POST /import/transfers` — validates cap, 64 MB part ceiling, contiguity, size sums, sha256 format), upload parts (`PUT .../parts/:n` — raw body, hash-and-size verified before an atomic write to a disk spool under the instance root; re-uploads are no-op successes), poll resume state (`GET .../:id` — missing parts recomputed from disk), and apply (`POST .../:id/apply` — requires all parts, re-verifies the assembled zip against the whole-file hash fail-closed, then feeds the existing import pipeline through factored helpers rather than duplicated logic). - Hourly sweep fails and cleans spools idle for 24 h; a swept transfer honestly reports all parts missing on resume. - Strict UUID gating on run ids before any filesystem path construction. - The existing single-shot upload path is untouched; clients arrive in the follow-up PR. ## Verification - Transfer route suite (embedded Postgres): create/upload/status/apply round-trip with a real imported company, out-of-order parts, wrong-hash part rejected and unrecorded, re-upload no-op, apply-with-missing-parts rejection, resume after failure with prior progress intact, assembled-hash mismatch failing closed with spool deletion, actor scoping 404s, async-job apply, sweep followed by honest resume. - Ledger suite (embedded Postgres): part idempotency, actor/direction scoping, completed-run short-circuit, cancelled runs staying cancelled. - Existing portability route suite unchanged and green; server + db typechecks clean. Exact counts in the PR checks. ## Risks - New routes are additive; the existing import path is untouched. The transfer routes carry the same board authorization as the import routes they sit beside. - Disk spool: bounded by the existing upload cap per transfer, cleaned on success, failure, hash mismatch, and by the 24 h sweep. Spool paths are strict-UUID-gated. - The apply step still materializes the assembled zip in memory once (same profile as today's single-shot import at apply time); upload-time memory drops to one part. - Known limitation, deliberate: transfers are keyed on content alone, so identical package content cannot be imported twice without re-exporting (surfaced explicitly to the caller). Acceptable for v1; noted for review. ## Model Used - Claude Fable 5 (`claude-fable-5`) via Claude Code CLI, with extended thinking and tool use (multi-agent implementation with independent verification). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting mergecanary/v2026.811.0-canary.16 |
||
|
|
2494a2a0fe |
perf: add repeatable issue-detail baseline rig (#10409)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The issue detail page is a core operator surface where perceived latency directly affects task navigation > - Performance work needs repeatable evidence so later optimizations can be compared against the same scenarios > - The page did not expose stable user-timing marks for its header or first useful content > - There was also no isolated seeded browser rig that measured warm navigation, cold deep links, waterfalls, or server time > - This pull request adds the instrumentation and a one-command Playwright baseline harness > - The benefit is that issue-page performance changes can be validated with reproducible median measurements instead of anecdotes ## Linked Issues or Issue Description **Subsystem affected** Cross-cutting: `ui/`, `server/`, and browser performance tooling. **Problem or motivation** The issue detail page performs a large client bootstrap and request fan-out, but the repository lacks stable user-timing boundaries and a repeatable benchmark. That makes performance changes difficult to compare and allows regressions to be judged from anecdotes instead of consistent evidence. **Proposed solution** Add stable header/content paint measures, development/QA-only lifecycle vital reporting, aggregate server timing for the issue endpoint, and a seeded Playwright command that runs warm/cold scenarios under throttled and unthrottled profiles with N≥5 median reporting. **Alternatives considered** Ad hoc DevTools recordings were rejected because they are not repeatable or reviewable. Production telemetry was rejected because this baseline should not change production data collection. A unit-only harness was rejected because it cannot capture browser bootstrap, rendering, and network waterfall costs. **Roadmap alignment** The roadmap calls for agent performance to be measurable over time. This change applies that evidence-first principle to a core operator page and does not duplicate a listed roadmap deliverable. **Additional context** The generated report includes warm and cold medians, TTFB/FCP/LCP where applicable, request and byte totals before first useful content, JavaScript bytes, and issue endpoint server timing. ## What Changed - Added `issue-detail:navigate→header-paint` and `issue-detail:navigate→content-paint` user-timing measures to the issue detail page. - Added development/QA-only TTFB, LCP, and INP console reporting without production telemetry delivery. - Added `Server-Timing` for `GET /api/issues/:id`. - Added `pnpm exec playwright test --config tests/perf/issue-detail/playwright.config.ts`, which seeds an isolated instance and runs N≥5 warm/cold samples under unthrottled and Fast 4G/4x CPU profiles. - Added Markdown, raw JSON, and Chrome-trace outputs with median baseline tables and waterfall data. ## Verification - `pnpm --filter @paperclipai/ui typecheck` - `pnpm --filter @paperclipai/server typecheck` - `pnpm check:token-gates` - `npx playwright test --config tests/perf/issue-detail/playwright.config.ts --list` - `pnpm exec playwright test --config tests/perf/issue-detail/playwright.config.ts` — passed 20 samples in 9.4 minutes (5 runs × 2 scenarios × 2 profiles) for the baseline; post-review integrity reruns also exercised the corrected paths, while this shared runner intermittently killed Chromium processes, so the rig now performs one bounded browser-crash retry per sample. - Baseline medians: warm unthrottled 278/447 ms header/content; cold unthrottled 646/646 ms; warm throttled 1240/2060 ms; cold throttled 3932/3933 ms. ## Risks - Low product risk: the new browser measurements are development/QA tooling and the UI timing work does not change visible layout. - `Server-Timing` exposes only aggregate handler duration, not query contents or private identifiers. - Native INP reporting uses supported browser event timing entries and silently no-ops where unsupported. > 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, GPT-5.4, tool-assisted coding and browser execution with reasoning enabled; context-window size is not exposed in this environment. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Dev Agent <dev@paperclip.ing>canary/v2026.811.0-canary.15 |