master
2804 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
11b863220d |
Merge ora-289-stale-run-young-threshold: skip young runs and surface wallClockSec
Docker / build-and-push (push) Failing after 28s
Release / verify_canary (push) Failing after 18m16s
Release / verify_stable (push) Has been skipped
Refresh Lockfile / refresh (push) Successful in 9m46s
Release / publish_canary (push) Has been skipped
Release / preview_stable (push) Has been skipped
Release / publish_stable (push) Has been skipped
|
||
|
|
e1251fae2d |
Merge pull request #1: fix(opencode-local): pad prompts below the opencode 1.16.2 silent-exit floor (ORA-284)
Docker / build-and-push (push) Failing after 27s
Release / verify_canary (push) Has been cancelled
Release / publish_canary (push) Has been cancelled
Release / verify_stable (push) Has been cancelled
Release / preview_stable (push) Has been cancelled
Release / publish_stable (push) Has been cancelled
Refresh Lockfile / refresh (push) Waiting to run
Merge pull request #1: fix(opencode-local): pad prompts below the opencode 1.16.2 silent-exit floor (ORA-284) |
||
|
|
d86062b321 |
fix(recovery): skip young runs (<30s) and surface wallClockSec on stale-run alerts (ORA-289)
The 0-byte / <200-byte run-log heuristic in stale_active_run_evaluation fires for both genuine stalls (ORA-284 / ORA-275 / ORA-276 class) and brand-new runs that have not yet produced output (class-1 false positives). Most firings were class-1, so operators could not trust the alert. - Add ACTIVE_RUN_OUTPUT_MIN_AGE_MS = 30s: runs younger than this are skipped regardless of log size, eliminating the class-1 noise. - Add ACTIVE_RUN_OUTPUT_LIKELY_ZOMBIE_THRESHOLD_MS = 5min with a near-empty log (<200 bytes) AND no spawn PID / no in-memory process handle, so genuine stalls fire at the operator's SLA instead of waiting for the 60-minute general suspicion threshold. - General 60-min suspicion and 4-h critical thresholds are unchanged. - Extract the rule into a pure decideStaleRunEvaluation() helper and thread the resulting decision into the issue body so wallClockSec and elapsedMs are visible on every new evaluation, with a 'Threshold applied' line explaining which branch fired. - Add unit tests covering: missing silence anchor, too-young, zombie, stall, near-empty boundary, both-process-anchors-present, general suspicion, critical escalation, and wallClockSec/elapsedMs reporting. |
||
|
|
cdca784e6d |
fix(opencode-local): pad prompts below the opencode 1.16.2 silent-exit floor (ORA-284)
PR / policy (pull_request) Successful in 51s
commitperclip PR Review / review (pull_request_target) Failing after 2m5s
PR / Typecheck + Release Registry (pull_request) Successful in 11m54s
PR / General tests (server (1/3)) (pull_request) Failing after 9m2s
PR / General tests (server (2/3)) (pull_request) Failing after 8m31s
PR / General tests (server (3/3)) (pull_request) Failing after 10m35s
PR / General tests (workspaces-a) (pull_request) Failing after 8m18s
PR / General tests (workspaces-b) (pull_request) Failing after 6m31s
PR / Build (pull_request) Successful in 13m27s
PR / Verify serialized server suites (1/4) (pull_request) Failing after 7m4s
PR / Verify serialized server suites (2/4) (pull_request) Failing after 8m12s
PR / Verify serialized server suites (3/4) (pull_request) Failing after 6m27s
PR / Verify serialized server suites (4/4) (pull_request) Failing after 6m15s
PR / Canary Dry Run (pull_request) Successful in 18m1s
PR / e2e (pull_request) Failing after 5m44s
PR / verify (pull_request) Failing after 6s
opencode 1.16.2 has a non-deterministic silent-exit mode that fires on short stdin prompts (observed 0-byte / <200-byte run logs, ~32% stall rate on FoundingEngineer wake-payload-only runs). The root cause lives in the opencode binary, not the adapter. As a server-side workaround, ensure every prompt is at least 256 bytes by appending a deterministic Paperclip Runtime Context block when the joined sections fall short. - New exported helper padShortPrompt(input) in execute.ts - New exported constant OPENCODE_PROMPT_MIN_LENGTH_CHARS = 256 - New promptMetrics.promptPadded (0/1) so the server can observe how often the workaround fires - Four new unit tests cover the long-prompt, short-prompt, empty-cwd, and trailing-newline paths Refs: ORA-284, ORA-275, ORA-276 Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
9ac24317e6 |
fix(issues): ignore cancelled child blockers in attention (#7577)
Docker / build-and-push (push) Failing after 3m33s
Refresh Lockfile / refresh (push) Successful in 10m42s
Release / verify_canary (push) Failing after 16m21s
Release / verify_stable (push) Has been skipped
Release / publish_canary (push) Has been skipped
Release / preview_stable (push) Has been skipped
Release / publish_stable (push) Has been skipped
Fixes #7578 ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies. > - Blocked issue health is surfaced through blockerAttention so operators can see whether blocked work has a live path. > - The blockerAttention graph uses both explicit blockedBy edges and direct child issue edges. > - Done children were already ignored, but cancelled direct children still appeared as unresolved blockers. > - Explicit cancelled dependencies should remain visible as dependency problems, but terminal direct children should not inflate a parent blocker count. > - This pull request narrows child-edge traversal to non-terminal children and adds a regression test for the observed case. > - The benefit is that cancelled child issues no longer make blocked parents look like they have extra unresolved blocker attention. ## What Changed - Added terminal child status filtering for blockerAttention parent-child traversal so cancelled direct children are ignored with done children. - Added a server regression test where a blocked parent has active explicit blockers plus a cancelled direct child; the cancelled child no longer increases unresolved counts or becomes the sample blocker. ## Verification - `perl -e 'alarm shift; exec @ARGV' 300 pnpm exec vitest run server/src/__tests__/issue-blocker-attention.test.ts` -> 19 tests passed. ## Risks - Low risk: the change only affects implicit direct-child blockerAttention edges. - Explicit blockedBy edges to cancelled issues are intentionally unchanged and remain represented as attention-required dependency problems. ## Model Used - OpenAI GPT-5, Codex coding agent in tool-enabled CLI environment; reasoning and code execution used for repository inspection, patching, git operations, and targeted test 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 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 (not applicable; no UI change) - [x] I have updated relevant documentation to reflect my changes (not applicable; bugfix covered by regression test) - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
d1e6662ed8 |
fix(server): let codex_local agents inherit host Codex login by default (#8425)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The `codex_local` adapter spawns the Codex CLI for an agent, and `server/src/routes/agents.ts` normalizes each agent's `adapterConfig.env` on create/hire/update > - PR #8272 added an isolation guard that, on every codex_local agent, force-set a per-agent `CODEX_HOME` and injected `OPENAI_API_KEY = ""`, and rejected any "shared" home > - This broke the common case: operators who deleted `CODEX_HOME` / `OPENAI_API_KEY` in the UI saw them silently re-appear on save, and every agent was forced into an isolated home instead of sharing the device's existing Codex login (`~/.codex` / `$CODEX_HOME`) > - This pull request replaces the always-on isolation guard with key-scoped isolation: a keyless agent gets no env overrides and inherits the host Codex login at runtime; we only carve out an isolated per-agent `CODEX_HOME` when the agent sets its own `OPENAI_API_KEY` > - The benefit is that env-var deletion now persists, agents on one device share the host login by default, and per-account isolation is still available by setting a per-agent key ## Linked Issues or Issue Description No public GitHub issue exists. Bug report (following `bug_report.yml`): **What happened?** In a `codex_local` agent's configuration, removing the `CODEX_HOME` and `OPENAI_API_KEY` env vars via the UI (clicking the X) appears to work, but on save they instantly re-appear. The persisted config never loses the slots. **Root cause.** `applyCodexLocalIsolationGuard` in `server/src/routes/agents.ts` (added in #8272) re-injected `CODEX_HOME` (defaulting to a per-agent home) and `OPENAI_API_KEY = ""` on every create/hire/update, and rejected shared homes outright. So a PATCH that omitted those keys had them written back server-side. **Expected behavior.** Deleting these env vars should persist. A codex_local agent with no key should inherit whatever Codex login already exists on the device. **Steps to reproduce.** 1. Open a `codex_local` agent's configuration with `CODEX_HOME` and `OPENAI_API_KEY` set. 2. Remove both env vars and save. 3. Re-open the config — both slots are back. Related (different approach): Refs #8399 (keeps per-agent isolation, fixes only the empty `OPENAI_API_KEY` slot), Refs #8272 (introduced the guard), Refs #8403 (managed-auth seeding into isolated homes). ## What Changed - `server/src/routes/agents.ts` — replaced `applyCodexLocalIsolationGuard` (+ `assertCodexLocalHomeIsNotShared` / `normalizeCodexLocalHomePath`) with `applyCodexLocalKeyIsolation`. A codex_local agent now receives **no** env overrides unless it explicitly sets `OPENAI_API_KEY`; only then is an isolated per-agent `CODEX_HOME` injected (and only if the agent has not set its own `CODEX_HOME`). Keyless agents fall back to the host Codex login at runtime. Dropped the now-unused `node:os` import and the shared-home rejection. - `server/src/__tests__/agent-adapter-validation-routes.test.ts` — updated the validation-route tests to assert env-var deletion persists, keyless agents get no overrides, and key-bearing agents still get an isolated `CODEX_HOME`. ## Verification ```bash cd server && npx vitest run src/__tests__/agent-adapter-validation-routes.test.ts ``` All 6 tests pass locally. Manually verified in the live UI: removing `CODEX_HOME` and `OPENAI_API_KEY` from a codex_local agent and saving now persists the deletion (slots no longer re-appear on reload). ## Risks - **Host-key leak for keyless agents on a host with `OPENAI_API_KEY` set.** Low/intended: the product direction here is that codex_local agents inherit the host's Codex login by default; per-account isolation is opt-in via a per-agent `OPENAI_API_KEY`, which then gets its own `CODEX_HOME`. - **Divergence from #8399.** That PR retains the per-agent isolation default and only stops injecting the empty key, so it does not address the `CODEX_HOME` re-injection half of this bug. This PR intentionally changes the default to host-login inheritance. Reviewers should pick one direction. - **No migration.** Existing agents that already store these slots are not auto-cleaned, but operators can now delete them and the deletion sticks. ## Model Used - Provider: Anthropic (Claude) - Model ID: `claude-opus-4-8` - Capabilities: tool use, code execution, extended reasoning, agentic workflow ## 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 - [ ] 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 - [ ] 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 |
||
|
|
2c98c8e1e5 |
Fix sandbox git publishing and large workspace uploads (#8422)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Sandbox and SSH runtimes need to preserve agent work across isolated execution environments > - Git-backed workspaces were being copied mostly as filesystem archives, which breaks when `.git` points outside the mounted workspace and makes sandbox agents unable to publish their own branches > - Large ignored dependency trees could also be swept into the sandbox overlay, causing multi-GB transfers and max-string failures in some sandbox clients > - This pull request makes sandbox runtime setup use a git-backed HEAD sync plus a small dirty/untracked overlay, and bounds sandbox file transfers so large archives do not need one huge string > - The benefit is that sandbox agents can commit and push from a usable git checkout without uploading dependency trees such as `node_modules` ## Linked Issues or Issue Description Refs #8395 No public duplicate issue or PR was found after searches for `sandbox git workspace`, `git push sandbox`, and `node_modules sandbox upload`. Bug report: - What happened: sandbox-backed agent workspaces could receive a `.git` file that pointed at host-only git state, leaving the sandbox unable to run normal git workflows. The sandbox overlay upload could also include ignored dependency directories, creating very large transfers. - Expected behavior: sandbox and remote runtimes should prepare a usable git-backed workspace, copy only the necessary workspace overlay, and restore git history plus file changes without depending on a host-only `.git` path. - Steps to reproduce: 1. Run an agent in a sandbox-backed workspace whose local git checkout is a worktree. 2. Ask the agent to complete a GitHub workflow that requires commit/push access. 3. Observe that git operations can fail inside the sandbox, and ignored dependency trees can be uploaded as part of the workspace overlay. - Paperclip version or commit: reproduced against `master` before this PR, base `7aa212296eb1`. - Deployment mode: local dev / sandbox-backed runtime. - Installation method: built from source. - Agent adapters involved: local adapters using shared adapter-utils runtime preparation. - Database mode: not database-related. - Access context: agent runtime. - Local verification environment: Node.js v25.6.1, pnpm 9.15.4, macOS arm64. - Privacy checklist: all pasted output was reviewed for secrets, private hostnames, local usernames, and internal instance links. ## What Changed - Added a GitHub workflow push preflight so agent runs can detect missing push credentials when a workflow explicitly needs GitHub publishing. - Added shared git workspace sync helpers for shallow HEAD import/export and dirty/untracked overlay tracking. - Updated sandbox managed runtime setup to use git history plus a selected overlay instead of uploading the full local workspace for git-backed workspaces. - Bounded sandbox archive upload/download paths so large payloads stream or chunk instead of materializing one oversized string. - Excluded `.git` and ignored dependency trees from sandbox upload, download, and restore baselines while preserving local ignored directories during sync-back. - Added focused tests for git workspace sync, sandbox overlay selection, transfer chunking, and heartbeat push-preflight behavior. ## Verification - `pnpm exec vitest run packages/adapter-utils/src/git-workspace-sync.test.ts packages/adapter-utils/src/sandbox-managed-runtime.test.ts packages/adapter-utils/src/command-managed-runtime.test.ts server/src/__tests__/heartbeat-project-env.test.ts server/src/__tests__/heartbeat-workspace-session.test.ts` - `pnpm --filter @paperclipai/adapter-utils typecheck` - `pnpm --filter @paperclipai/server typecheck` - `pnpm build` - Public-hygiene scan of the PR diff and commit messages for internal issue ids, local paths, private hostnames, and obvious token patterns. ## Risks - Medium risk: this changes sandbox runtime synchronization semantics for git-backed workspaces, especially around dirty tracked files, untracked files, deleted paths, and ignored files. - The main mitigation is focused test coverage for upload contents, restore exclusions, and git round-trip behavior. - The SSH runtime keeps the current bundle-based implementation from `master`; this PR only aligns shared excludes and sandbox behavior with that model. ## Model Used OpenAI Codex, GPT-5-based coding agent, tool-enabled shell/git/GitHub workflow, with code execution and repository inspection. ## 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] 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] 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: Paperclip <noreply@paperclip.ing> |
||
|
|
33353ce62b |
feat(skills): remove bundled paperclip-dev skill and retire required skill attribute (#7029)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Local adapters (Claude, Codex, Cursor, Gemini, Grok, OpenCode, Pi, ACPX) ship bundled "skills" — opinionated Markdown prompt bundles materialized into the agent's runtime > - One of those bundled skills, `paperclip-dev`, existed to let agents develop Paperclip itself; it has now moved to its own external repo and no longer belongs in the core tree > - The adapter skill model also carried a `required` / `requiredReason` attribute plus a `paperclip_required` `AdapterSkillOrigin` variant, all of which only existed to mark bundled skills as non-optional in the UI and adapter sync logic > - With `paperclip-dev` gone, no bundled skill is "required" anymore, and the type / runtime surface for `required` is dead weight — but it is computed at request time and never persisted, so a clean removal is safe (no compatibility shim needed) > - This pull request deletes `skills/paperclip-dev/` and removes every trace of the `required` / `requiredReason` field and the `paperclip_required` origin across shared types, validators, adapter-utils, all eight local adapters, server routes, the company-skills service, the UI, the storybook fixtures, and the test suite > - The benefit is a smaller, simpler adapter-skill surface: one origin (`company_managed`) for managed bundled skills, `resolvePaperclipDesiredSkillNames` collapses to "just the configured desired set", and the AgentDetail skills tab no longer renders a "Required by Paperclip" section that no longer applies ## Linked Issues or Issue Description <!-- No existing public GitHub issue; describing the underlying work inline (feature_request template fields). --> **Summary** Remove the bundled `paperclip-dev` skill (now maintained in its own external repo) and retire the `required` / `requiredReason` skill attribute and the `paperclip_required` skill origin, which only existed to support it. **Problem or motivation** `paperclip-dev` is the only bundled skill that was ever marked "required". Now that it lives in a separate repository, shipping it inside the core tree is wrong, and the entire `required` surface (a type field, a validator field, a synthesized `paperclip_required` origin, UI "Required by Paperclip" section, and required-skill merging in the desired-skills calculation) becomes dead weight. The `required` value is computed at request time and never persisted, so it can be removed cleanly without a migration or compatibility shim. **Proposed solution** Delete `skills/paperclip-dev/`, drop the `required` / `requiredReason` fields and `paperclip_required` origin everywhere they are produced or consumed, collapse managed-skill origin to a single `company_managed` value, and simplify `resolvePaperclipDesiredSkillNames` to return only the configured desired set. **Alternatives considered** Keeping the `required` attribute as a no-op for forward compatibility — rejected because it is request-time only (nothing persists it), so leaving it in place is pure dead surface area with no callers. **Roadmap alignment** Internal cleanup / dead-code removal that simplifies the adapter-skill surface; it does not introduce or duplicate any planned core feature in ROADMAP.md. ## What Changed - Deleted bundled `skills/paperclip-dev/` (moved to a separate repo). - Dropped `required`, `requiredReason`, and the `paperclip_required` origin from `packages/shared/src/types/adapter-skills.ts`, `packages/shared/src/validators/adapter-skills.ts`, and `packages/adapter-utils/src/types.ts`. - In `packages/adapter-utils/src/server-utils.ts`: removed `readSkillRequired()`; dropped `required`/`requiredReason` from `listPaperclipSkillEntries()`, `normalizeConfiguredPaperclipRuntimeSkills()`, `buildPersistentSkillSnapshot()`, and `PaperclipSkillEntry`; collapsed `buildManagedSkillOrigin()` to always return `company_managed`; simplified `resolvePaperclipDesiredSkillNames()` to return only the configured desired set (signature preserved so adapter call sites are untouched). - Walked all eight local adapters (`acpx-local`, `claude-local`, `codex-local`, `cursor-local`, `gemini-local`, `grok-local`, `opencode-local`, `pi-local`) and removed every remaining `requiredReason` / `paperclip_required` reference. - `server/src/services/company-skills.ts`: dropped the `required = sourceKind === "paperclip_bundled"` synthesis when listing runtime skill entries. - `server/src/routes/agents.ts`: removed required-skill merging from the desired-skills calculation in the persist-config path and the unsupported-snapshot path (keeping the current version-aware `desiredSkillEntries` structure). - `ui/src/pages/AgentDetail.tsx`: dropped required-based filters, the required tooltip, and the entire "Required by Paperclip" section from the agent skills tab; storybook fixtures in `ui/storybook/stories/acpx-local.stories.tsx` cleaned up to match. - Tests: deleted the `required: false` case in `paperclip-skill-utils.test.ts` and the "keeps required bundled skills installed" case in every `*-local-skill-sync.test.ts`; `acpx-local-execute.test.ts`, `cursor-local-execute.test.ts`, `cursor-local-skill-sync.test.ts`, `agent-skills-routes.test.ts`, and `packages/adapter-utils/src/server-utils.test.ts` were updated to drop removed fields and map `origin: "paperclip_required"` → `"company_managed"`. - `server/src/adapters/registry.ts`: two `as unknown as ServerAdapterModule["..."]` casts on `hermesListSkills` / `hermesSyncSkills` (matching the existing `executeHermesLocal` pattern). `hermes-paperclip-adapter@0.2.0` still depends on the published `@paperclipai/adapter-utils` which keeps the retired `paperclip_required` variant; the cast bridges the workspace-vs-published type mismatch at the registry seam and can drop once hermes upgrades. ## Verification Run from the workspace root: ```sh grep -rn "skills/paperclip-dev" . grep -rn "paperclip_required" --include="*.ts" --include="*.tsx" . grep -rn "requiredReason" --include="*.ts" --include="*.tsx" . pnpm -w typecheck pnpm --filter @paperclipai/server exec vitest run paperclip-skill-utils pnpm --filter @paperclipai/server exec vitest run skill-sync ``` The first three greps return only the explanatory comment in `server/src/adapters/registry.ts` (no live `paperclip_required` / `requiredReason` usage) and zero `skills/paperclip-dev` source hits. Locally: - `pnpm -w typecheck` → all packages this PR touches pass (adapter-utils, shared, server, ui, cli, and the cursor/gemini/opencode/pi adapters). - Affected vitest suites pass: `paperclip-skill-utils`, `server-utils`, all eight `*-local-skill-sync`, `agent-skills-routes`, and the `acpx`/`cursor`/`pi` execute suites. ## Risks - Behavioral shift in the agent skills UI: the "Required by Paperclip" section disappears. No bundled skill is required anymore, so this only affects environments that previously surfaced `paperclip-dev` as a forced-on row; those installs will see the skill move into the regular "company-managed" list (and be uninstalled on next sync unless explicitly listed as desired). - Existing agents may still have the string `"paperclip-dev"` in their persisted `desiredSkills`. That entry is inert (no source for it to install from); a one-time DB cleanup is out of scope. Low risk. - Hermes adapter type bridge: two casts in `registry.ts` paper over a type-only divergence between the workspace `@paperclipai/adapter-utils` and the published version still pinned by `hermes-paperclip-adapter@0.2.0`. Runtime behavior is unaffected because the retired `paperclip_required` value is no longer produced by anything in this tree. The casts can be removed once hermes upgrades its dependency. ## Model Used - Provider: Anthropic - Model: Claude Opus 4.7 (`claude-opus-4-7`) - Capability: agent tool use via Paperclip's `claude_local` adapter ## 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: Paperclip <noreply@paperclip.ing> |
||
|
|
7aa212296e |
codex_local adapter output inactivity monitor (#5017)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - The `codex_local` adapter runs `codex exec` as a child process and streams JSONL events from its stdout > - NEE-79 caught a real-world `codex_local` orphan: codex sat in `read()` on stdin for 1h+, no JSON events emitted past startup, no LLM call in flight. The adapter had no inactivity timer; the only safety net was the platform-level 1h silent-run detector > - This is precisely the failure shape NEE-80 scoped: an adapter-detected fault that should be killed *by the adapter* and surfaced as `failed`, not waited out for an hour > - This pull request adds an output-inactivity watchdog inside the codex-local adapter that resets on every parsed JSONL event from stdout, kills the child via SIGTERM → 5s grace → SIGKILL when it fires, and resolves the run with a structured watchdog failure > - The benefit is that NEE-79-class hangs shorter than 1h stop reaching the platform-level safety net — they fail fast and visibly at the adapter layer, with diagnostic logs that don't require host shell access ## Linked Issues or Issue Description No existing GitHub issue covers this; describing the underlying bug in-PR per the bug report template (`.github/ISSUE_TEMPLATE/bug_report.yml`): - **What happened?** A `codex_local` run sat in `read()` on stdin for over an hour: codex emitted its startup JSONL events, then nothing — no further events, no LLM call in flight, no exit. The adapter kept the child alive indefinitely; the run was only reaped by the platform-level 1h silent-run safety net, an hour after it had effectively died. - **Expected behavior:** The adapter should detect that its child has stopped producing output long before the platform-level safety net, kill it, and surface the run as `failed` with a diagnostic that explains what happened. - **Steps to reproduce:** Run any `codex_local` issue where `codex exec` hangs after startup (e.g. codex blocks reading stdin and never emits another JSONL event). Observe the run stays alive until the 1h platform safety net fires. - **Paperclip version or commit:** reproduced on master prior to this branch. - **Agent adapter(s) involved:** codex_local. Related PRs found while searching for duplicates (none implement an event-aware inactivity watchdog inside `codex_local`): - #4004 — generic idle + wall watchdogs for `runChildProcess` in adapter-utils; complementary, operates below the JSONL parse layer and is not codex-event-aware - #4742 — fail-fast on codex-local *startup* hang; this PR covers the post-startup hang class - #6861 — zombie-run termination for codex-local; reaping after exit, not inactivity detection - #7811 — the analogous output-idle timeout for grok-local ## What Changed - `packages/adapters/codex-local/src/server/watchdog.ts` *(new)* — pure watchdog primitive with injectable timers/clock: `resolveCodexInactivityTimeout`, `createCodexInactivityWatchdog`, `formatWatchdogErrorMessage`. Default `7 * 60_000` ms; honors `null` as the disabled escape hatch - `packages/adapters/codex-local/src/server/execute.ts` — `runAttempt` now wraps `onSpawn` to capture `pid`/`processGroupId`, feeds stdout chunks through `noteStdoutChunk`, and on watchdog fire sends SIGTERM to the process group, schedules SIGKILL after 5s, and returns an `AdapterExecutionResult` with `exitCode: null`, `signal: SIGTERM|SIGKILL`, `errorMessage: "watchdog: no codex output for {N}m {S}s"`, `errorCode: "codex_output_inactivity_watchdog"`. With `errorMessage` set and `timedOut: false`, `heartbeat.ts:5860` maps the run to `outcome === "failed"` (not `cancelled`) - `packages/adapters/codex-local/src/index.ts` — `agentConfigurationDoc` documents `outputInactivityTimeoutMs` (number ms; `null` disables; non-positive falls back to default with a warning log at spawn) - `packages/adapters/codex-local/src/server/watchdog.test.ts` *(new)* — 13 tests covering acceptance criteria 2 and 3 plus supporting cases: fires after silence, no-fire across 12× (threshold − 1s) cycles, multi-event chunks, non-JSON ignoring, single-fire idempotency, formatter shape, full resolution table, `null` → disabled - `packages/adapters/codex-local/src/server/watchdog.integration.test.ts` *(new)* — real Node subprocess that prints one JSONL event then sleeps; `runChildProcess` reaps it within `threshold + 6s`; signal is SIGTERM or SIGKILL; `parsedEventCount === 1` (acceptance criteria 1 and 4) ## Verification ``` $ pnpm --filter @paperclipai/adapter-codex-local typecheck > tsc --noEmit # clean $ pnpm --filter @paperclipai/adapter-codex-local exec vitest run ✓ src/server/quota-spawn-error.test.ts (1 test) ✓ src/server/codex-home.test.ts (3 tests) ✓ src/server/codex-args.test.ts (3 tests) ✓ src/server/watchdog.test.ts (13 tests) ✓ src/server/parse.test.ts (9 tests) ✓ src/server/execute.remote.test.ts (4 tests) ✓ src/ui/parse-stdout.test.ts (3 tests) ✓ src/ui/build-config.test.ts (1 test) ✓ src/server/watchdog.integration.test.ts (1 test) Test Files 9 passed (9) Tests 38 passed (38) ``` Acceptance criteria check: 1. ✅ Simulated child emits one event then sleeps → killed at threshold; result `errorMessage` matches `watchdog: no codex output for {N}m {S}s` (`watchdog.integration.test.ts`) 2. ✅ Child emits events every (threshold − 1s) → not killed (`watchdog.test.ts: "does not fire when events arrive every (threshold - 1s)"`) 3. ✅ `outputInactivityTimeoutMs: null` disables the watchdog (`resolveCodexInactivityTimeout` returns `disabled`; `execute.ts` skips watchdog construction and logs a startup warning) 4. ✅ Real subprocess reaped well within `threshold + 6s` — 250 ms threshold, 290 ms wall clock in CI 5. ✅ Adapter-level fault → `outcome === "failed"` per `heartbeat.ts:5860`. NEE-79-class hangs <1h get caught at the adapter, not the platform-level safety net Post-rebase verification (head `2a8703fab`, rebased onto master `69a368ed5`): ``` $ pnpm --filter @paperclipai/adapter-codex-local typecheck # clean $ pnpm --filter @paperclipai/adapter-codex-local exec vitest run Test Files 11 passed (11) Tests 64 passed (64) ``` ## Risks - Low risk. Behind a default-on watchdog that only fires after 7m of zero parsed JSON events. Operators can disable it with `outputInactivityTimeoutMs: null` for known-slow tasks - The kill path reuses the same `process.kill(-pgid, signal)` pattern that `runChildProcess` already uses for its terminal-result cleanup, so signal semantics match the existing code path - `timedOut: false` is preserved on watchdog fire — the platform-level timeout outcome is unchanged, only the `failed`-vs-success classification flips. No behavioral shift for already-failing runs - Sandbox/SSH execution targets: the watchdog fires and emits the structured log, but the kill is best-effort because remote pids aren't owned by this process. The platform-level 1h safety net still applies. Out of scope for NEE-81 by design ## Model Used - Provider: Anthropic Claude - Model: claude-opus-4-7 - Mode: Claude Code (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 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 changes) - [x] I have updated relevant documentation to reflect my changes (`agentConfigurationDoc` for `outputInactivityTimeoutMs`) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (re-running on the rebased head) - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (P2 addressed; review threads resolved) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Neeraj Kumar Singh <b.nirajkumarsingh@hotmail.com> Co-authored-by: Devin Foley <devin@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
5ebbe67ebd |
fix(workspace-runtime): base fresh worktrees on origin/master and refresh unstarted reuses (#8412)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent work runs inside git worktrees created by the workspace runtime, each branched from a configured base ref (typically the repo's default branch) > - When the local `master` is stale or ahead of `origin/master` (committed but unpushed work, or local-only commits), a freshly created worktree inherits that divergence — so an unrelated task branch silently carries commits it never intended to touch > - This surfaced as a docs-only task whose PR accidentally pulled in unrelated changes from a diverged local master > - The base for a fresh worktree should be resolved authoritatively to the remote-tracking ref (`origin/<branch>`), and an idle/unstarted reused worktree should be safely fast-forwarded — without ever destroying in-progress work > - This pull request makes both behaviors explicit in the workspace runtime > - The benefit is that task branches start from a clean, authoritative base, eliminating accidental inclusion of unrelated local changes ## Linked Issues or Issue Description This is a bug fix. No public GitHub issue exists, so describing it inline following the Bug Report template: **What happened** A task intended to change only docs produced a PR that also contained unrelated changes pulled in from `master`. The root cause: when a new worktree is created from a configured local branch (e.g. `master`), the worktree inherits whatever that local branch points at. If the local `master` has committed divergence from `origin/master` (unpushed or local-only commits), that divergence leaks into the new task branch. The leak comes from committed local-vs-`origin/master` ref drift, not uncommitted working-tree changes (each worktree has its own working tree). **Expected behavior** A fresh worktree should be based on the authoritative `origin/master` head so unrelated local commits never seed a task branch. **Steps to reproduce** 1. Have a local `master` that is ahead of `origin/master` (committed but unpushed work). 2. Create a new worktree/task branched from `master` via the workspace runtime. 3. Open a PR from that branch — it carries the unrelated local commits. **Deployment mode** Self-hosted / local workspace runtime. ## What Changed - Fresh worktrees now resolve their base ref authoritatively: a configured local branch (e.g. `"master"`) is mapped to its `origin/<branch>` remote-tracking counterpart so unpushed/ahead local commits can never seed a task branch. Remote-tracking refs, SHAs, and tags are used verbatim; an unset/`HEAD` base falls back to the detected default branch. The resolved ref is recorded (`repoRef`) so downstream drift checks stay accurate. - If a configured local branch has no matching `origin/<branch>`, the runtime warns and falls back to the local ref rather than failing. - On reuse, a *provably unstarted* worktree (no commits past base + clean tree including untracked files) is fast-forwarded to the latest `origin/master`. Started or dirty worktrees keep the prior warn-only behavior, so in-progress work is never reset. Only remote-tracking bases are eligible for the refresh. ## Verification - `cd server && npx vitest run src/__tests__/workspace-runtime.test.ts` - 5 new tests cover: local-branch→`origin/<branch>` mapping, no-remote fallback warning, unstarted-reuse fast-forward, and that started/dirty worktrees are left untouched. - Result: 65 passed. 1 pre-existing failure (`auto-detects the default branch via symbolic-ref when origin/HEAD is set`) is unrelated to this change and fails only due to the test host's git default-branch config (test setup runs `git push -u origin main master` but the local default branch is `main`); it also fails on `master`. ## Risks - Low risk. The refresh path is intentionally conservative: it only fast-forwards worktrees that are provably unstarted (zero commits past base and a fully clean tree, including untracked files) and only when the base is a remote-tracking ref. Started or dirty worktrees fall through to the existing warn-only drift behavior, so no in-progress work can be destroyed. - Behavioral shift: fresh worktrees configured against a local branch will now base on `origin/<branch>` instead of the local ref. This is the intended fix; the only case it changes is when local and remote have diverged. ## Model Used Claude — `claude-opus-4-8` (extended thinking, tool use / 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 (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 - [ ] If this change affects the UI, I have included before/after screenshots (N/A — no UI changes) - [x] I have updated relevant documentation to reflect my changes (N/A — no doc changes needed) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending CI run) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending review) - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
a937b89a47 |
fix(daytona): valid memory input in env config form + size presets (#8389)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Environments are configured through a JSON-schema-driven form (`JsonSchemaForm`), and each sandbox provider (here, Daytona) supplies a manifest describing its fields > - In the Daytona environment config form, the "memory" field rendered as a free-text input; on save the value round-tripped to `0`, producing a server-side "number must be greater than zero" error even when the user typed a valid number like `2` > - Rather than patch the free-text input, memory is now a true dropdown of the sandbox sizes Daytona actually supports, so an invalid value can no longer be typed or coerced > - The dropdown leads with a blank "None" row that is selected by default (meaning "not configured — use Daytona's defaults"); `0` is never offered because it is not a valid configuration > - The benefit is that operators pick a valid memory size from a constrained list, it saves correctly as an integer, and leaving it unset cleanly omits the field instead of submitting `0` ## Linked Issues or Issue Description No public GitHub issue exists; describing the bug inline per the bug report template. ### What happened? In the Daytona environment configuration form, the memory field is a free-text input labelled "gigabytes of RAM". Entering a value such as `2` and saving coerced the value to `0`, and the server rejected the save with "the number must be greater than zero". There was no way to enter a valid memory size through the form. ### Expected behavior Selecting a valid memory size (e.g. `2`) keeps that value and saves cleanly as an integer. Leaving memory unset is valid and submits no value (Daytona defaults apply). `0` is never selectable. ### Steps to reproduce 1. Open the environment configuration form for a Daytona sandbox provider. 2. In the memory field, type `2`. 3. Click Save. 4. Observe the value becomes `0` and the form errors with "the number must be greater than zero". ## What Changed - Daytona manifest: `memory` is now an `enum` of the supported sandbox sizes `[1, 2, 4, 8]` (GiB). It stays optional, so "not configured" remains valid. `0` is not in the list. - `JsonSchemaForm` `EnumField`: optional enums now render a leading blank **None** row that is selected by default when no value is set, letting the user express "not configured" and clear a previous selection (Radix `Select` forbids an empty-string item value, so the unset state maps to a sentinel that translates back to `undefined`). - `JsonSchemaForm` `EnumField`: when every enum option is numeric, the selected value is coerced back to a number on change so the payload keeps the schema's integer type (a stringified `"2"` would otherwise fail server-side integer validation — this is what fixes the original bug for the dropdown path). - Tests: added `EnumField` coverage in `JsonSchemaForm.test.tsx` (blank row present, no `0`, blank selected by default, numeric coercion, blank → unset) and Daytona manifest coverage in the plugin test (memory enum is `[1,2,4,8]`, excludes `0`, stays optional). ## Verification - `cd ui && npx vitest run src/components/JsonSchemaForm.test.tsx src/pages/CompanyEnvironments.test.tsx` → 16/16 passing (14 + 2). - `vitest run` in `packages/plugins/sandbox-providers/daytona` → 19/19 passing (incl. 3 new manifest tests). - `cd ui && npx tsc --noEmit` → clean. - Behavior: the memory field now renders as a dropdown showing **None / 1 / 2 / 4 / 8**, with **None** selected by default. Picking `2` stores the integer `2`; picking **None** clears the field so it is omitted from the payload (no `0`, no "must be greater than zero" error). ## Risks - Low risk. The blank-row + numeric-coercion changes live in `EnumField`. The blank row is only added for **optional** enums (required enums are unaffected); numeric coercion only triggers when every option is numeric, so existing string enums (`egressMode`, `backend`, `sessionStrategy`) are unchanged. The manifest change is scoped to the Daytona provider. ## Model Used Claude (Anthropic), Opus-class model, used via the Paperclip agent workflow (author + reviewer agents) with 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 (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 - [x] My branch name describes the change and contains no internal Paperclip ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
254b01d2af |
docs: drop contributor screenshot requirement in favor of cutter.sh bot (#8409)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors follow `CONTRIBUTING.md` and the PR template when opening pull requests > - Those docs still tell contributors to manually capture and attach before/after screenshots for UI changes > - That requirement is now redundant: the `cutter.sh` bot automatically captures and posts before/after UI screenshots to every PR > - Asking contributors to also do it by hand adds friction and produces duplicate screenshots > - This pull request removes the manual screenshot obligation from `CONTRIBUTING.md` and the PR template, and documents that `cutter.sh` handles it > - The benefit is a clearer, lower-friction contribution flow that matches how screenshots actually get posted today ## Linked Issues or Issue Description No public GitHub issue. Underlying change: Our contributor docs and PR template instructed contributors to attach before/after screenshots for any UI change. We now run a bot (`cutter.sh`) that automatically captures and posts before/after UI screenshots to the PR, so the manual step is no longer needed. This change removes the manual requirement and instead documents that the bot handles screenshots, asking contributors to simply describe the visible change. ## What Changed - `CONTRIBUTING.md`: removed the "Before / After screenshots" requirement from the Path 2 PR checklist and from the "Writing a Good PR message" section; both now explain that the `cutter.sh` bot posts UI screenshots automatically and ask contributors to describe the visible change instead. - `.github/PULL_REQUEST_TEMPLATE.md`: updated the Verification guidance to note that `cutter.sh` posts before/after screenshots automatically, and removed the "include before/after screenshots" item from the checklist. - Issue templates were intentionally left unchanged — they only offer screenshots as an optional "if helpful" field, never a requirement. ## Verification - Docs-only change; no code paths affected. - `grep -rin "screenshot" CONTRIBUTING.md .github/` confirms no screenshot *requirement* remains — only the new `cutter.sh` wording and the optional "if helpful" fields in issue templates. - Render `CONTRIBUTING.md` and the PR template to confirm the wording reads cleanly. ## Risks Low risk — documentation-only change. No build, runtime, or migration impact. ## Model Used Claude (Anthropic), model `claude-opus-4-8`, 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 (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (docs-only; no tests apply) - [ ] I have added or updated tests where applicable (N/A — docs-only) - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending CI) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending review) - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
67ca8287e8 |
fix(codex-local): seed managed auth into isolated homes (#8403)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The `codex_local` adapter isolates local Codex runs by assigning managed `CODEX_HOME` state per company and per agent > - PR #8272 tightened that isolation, but it left a gap: once `CODEX_HOME` became explicit, the adapter treated it like a user-managed override and skipped auth seeding > - That meant newly isolated agents could launch with no usable `auth.json`, hit OpenAI unauthenticated, and fail with `401 Missing bearer` > - Users who had already persisted one of those broken managed homes could remain stranded even after config changes unless Paperclip repaired the home itself > - This pull request teaches Paperclip to seed managed homes correctly, backfill already-stranded managed homes on startup, and reject credential-less managed homes before they reach the provider > - The benefit is that affected managed `codex_local` agents recover automatically after upgrade and restart, without manual `CODEX_HOME` surgery ## Linked Issues or Issue Description - Fixes #497 - Refs #5028 - Related PR: #8272 - Related PR: #8399 ## What Changed - Distinguished Paperclip-managed `CODEX_HOME` paths from genuine external overrides and always seeded auth into managed homes, even when `CODEX_HOME` is explicit in config. - Wrote API-key-backed `auth.json` files for managed homes when `OPENAI_API_KEY` is configured, otherwise symlinked the shared Codex auth for subscription/OAuth flows. - Added a startup reconciliation pass that backfills already-isolated managed homes created by the broken release so upgrade plus restart repairs stranded agents automatically. - Preserved previously resolved API-key auth when the stored `OPENAI_API_KEY` binding is secret-backed and startup cannot resolve the secret value directly. - Hardened the managed-home preflight to require a credential-bearing `auth.json`, not just file presence, and documented the recovery behavior. - Added regression tests covering managed-home seeding, fail-fast behavior, and server-side startup reconciliation. ## Verification ```bash pnpm exec vitest run packages/adapters/codex-local/src/server/codex-home.test.ts packages/adapters/codex-local/src/server/execute.auth.test.ts server/src/__tests__/codex-auth-reconciliation.test.ts server/src/__tests__/codex-local-execute.test.ts server/src/__tests__/server-startup-feedback-export.test.ts pnpm --filter @paperclipai/adapter-codex-local typecheck pnpm --filter @paperclipai/server typecheck pnpm check:tokens pnpm exec vitest run server/src/__tests__/server-startup-feedback-export.test.ts ``` GitHub Actions `PR` workflow is green on latest head `e5b1e08d4`, including policy, typecheck, test shards, build, e2e, serialized server suites, and canary dry run. Greptile Review is green on latest head with 0 comments added. No UI changes. ## Risks - Startup reconciliation now mutates persisted managed Codex homes at boot. Risk is low because it only touches Paperclip-managed company/agent home paths and no-ops when a home already has usable auth. - Genuine external `CODEX_HOME` overrides remain intentionally self-managed, so those users still own repair steps inside their custom home. - Hosts with neither shared Codex auth nor an explicit per-agent API key now fail earlier with a clearer adapter error instead of surfacing a downstream `401`, which changes timing but not capability. ## Model Used - OpenAI Codex, GPT-5-based coding agent in a local Codex session; exact served model ID/context window were not exposed to the session. Tool use and code execution were enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [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 - [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] 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> |
||
|
|
e281095cb4 |
fix(ui): make adapter test button test only (#8405)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent adapter configuration is where operators edit the runtime settings that control how each agent is invoked. > - The adapter form includes a test action so operators can validate the in-progress configuration before saving it. > - In edit mode, that test action changed into `Save + Test` when the form was dirty, which mixed validation with persistence. > - Operators already have an explicit Save action for committing adapter changes. > - This pull request makes the adapter test action test-only so validation never implicitly saves a dirty draft. > - The benefit is clearer operator control: Test validates the current draft, Save persists it. ## Linked Issues or Issue Description No matching public GitHub issue or in-flight PR was found, so this PR describes the bug inline. ### Pre-submission checklist - I searched existing open and closed issues and did not find a duplicate. - I can reproduce this behavior on current `master` before this branch. - I confirmed this originates in the Paperclip UI, not in a specific local agent CLI or provider configuration. ### What happened? When editing an existing agent adapter configuration, changing an adapter setting made the adapter environment test button change from `Test` to `Save + Test`. Clicking it saved dirty edits before running the environment test. ### Expected behavior The test action should always be a test action. Dirty draft values should be validated without persisting them, and the explicit Save button should be the only save path. ### Steps to reproduce 1. Open an existing agent adapter configuration. 2. Edit an adapter setting so the form becomes dirty. 3. Observe the adapter environment test action label and behavior. ### Paperclip version or commit Current `master` before this branch, `c0743482b`. ### Deployment mode Local dev/private operator UI. ### Installation method Built from source with pnpm. ### Agent adapter(s) involved Not adapter-specific. The behavior lives in the shared agent adapter config form. ### Database mode Not database-related. ### Access context Board operator UI. ### Additional context Public searches performed before opening this PR: - `adapter test save` - `Save + Test` - `AgentConfigForm testEnvironment` - `adapter configuration test button` No logs or local config are needed for this UI-only behavior report. ## What Changed - Removed the helper that converted dirty edit-mode adapter tests into `Save + Test`. - Removed the save-before-test helper path so clicking Test only invokes the adapter environment test mutation. - Kept draft testing behavior by continuing to build the test payload from the current in-progress adapter config. - Dropped unit tests that asserted the old save-and-test behavior. ## Verification - `pnpm vitest run ui/src/components/AgentConfigForm.test.ts` -> 6/6 passed. - `pnpm --filter @paperclipai/ui typecheck` was attempted locally and failed on the existing unrelated `packages/plugins/sdk/src/ui/components.ts` React module-resolution error before reaching this change. - Static review: `Save + Test`, `getAgentConfigTestActionLabel`, and `runAgentConfigEnvironmentTest` no longer appear in `ui/src`. - Before/after visual state for the dirty edit-mode adapter test action: - Before: `Save + Test` - After: `Test` - PR CI passed: policy, commitperclip review, typecheck/release-registry, build, general tests, serialized server suites, e2e, canary dry run, aggregate verify, and security scans. - Greptile passed with 5/5 confidence. The remaining screenshot/documentation thread was answered and resolved because this is a text-only button-label state already captured above. ## Risks Low risk. This is a narrow UI behavior change in the adapter config form. The main behavior change is intentional: testing a dirty adapter draft no longer persists it. If an operator expected Test to save edits, they must now use the explicit Save button. ## Model Used - Anthropic Claude via Paperclip `claude_local` produced the initial implementation commit. - OpenAI GPT-5 Codex via Paperclip `codex_local` reviewed the change, amended public commit metadata, pushed the branch, and prepared this pull request. The adapter did not expose an exact context-window value in the run payload; tool use and local shell execution were used. ## 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] 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] 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> |
||
|
|
ac143f3254 |
style(ui): simplify Company Environments screen copy (#8400)
## Thinking Path > - Paperclip is the control plane for managing AI-agent work, and the UI needs to stay legible because operators use it to steer execution. > - This change sits in the Company Environments screen inside the Paperclip app UI. > - The screen carried redundant headings and several overlapping helper sentences that added clutter without adding meaning. > - The request was presentation-only: remove redundant copy and simplify labels, with no change to environment-selection behavior. > - The safest path was a single-file edit in `CompanyEnvironments.tsx` that preserved every control and only removed/renamed text. > - This is a `style:` change (UI copy/presentation), so no behavior is altered and no new test is warranted. ## Linked Issues or Issue Description No public GitHub issue exists for this small UI cleanup, so the problem is described here instead. - The Company Environments screen had redundant headings and overlapping helper copy. - The default environment control used a verbose label and a Local option label that added noise without adding meaning. - Requested outcome: simplify the copy so the screen is easier to scan while preserving the same behavior. Related public PRs reviewed for overlap: - Refs #8380 - Refs #8391 - Refs #8398 - Refs #4902 ## What Changed - Removed the extra top-level `Environments` header inside the screen content (both enabled and disabled states). - Removed the introductory paragraph above the environment list. - Renamed `Instance default environment` to `Default` and removed its helper sentence. - Removed the duplicate helper text below the default environment select. - Changed `Local (built-in default)` to `Local`. - Removed the redundant `Saved environments` section heading. - Removed the empty-state line `No saved environments yet. Local remains the default until you add another target.` - Removed the previously added UI test, which only asserted the now-final copy and added no lasting value. - Removed the now-unused `Settings` import and `instanceDefaultEnvironment` variable. ## Verification - Ran the existing `CompanyEnvironments` vitest suite locally: 4 passed. - This is a presentation-only copy change with no API, state, or behavior changes. - Reviewer can open Company Settings -> Environments and confirm the screen matches the requested wording cleanup with no behavior change. ## Risks - Low. Copy-only UI change with no API or state-management changes. - The only real risk is removing more context than intended from the top of the screen; review should focus on clarity. > 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 - Anthropic Claude Opus 4.8 via the Claude local adapter with tool use in a Paperclip-managed workspace. ## 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 and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] This is a `style:` copy-only change; no new test is warranted (test-coverage gate skips `style:`) - [ ] If this change affects the UI, I have included before/after screenshots - [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 |
||
|
|
d5ceb82571 |
Prefer loopback runtime API URL for local agents (#5102)
## Thinking Path > - Paperclip is the open-source app that manages AI agents and their work coordination > - The server subsystem exposes a control-plane HTTP API that agents read and write task state against at runtime > - On local development machines, Paperclip binds to loopback (`127.0.0.1:3100`) but also advertises LAN hostnames via `allowedHostnames` for multi-device setups > - Agents inherit their API URL from `PAPERCLIP_API_URL` / `PAPERCLIP_RUNTIME_API_URL` env vars exported during server startup > - `choosePrimaryRuntimeApiUrl` was selecting the first entry in `allowedHostnames` (the LAN IP) before the loopback bind host, so agents on the same machine tried to connect to an unreachable LAN address (NEE-327) > - This PR fixes the chooser to return the normalized loopback bind host first, before considering LAN `allowedHostnames` > - The benefit is that local agents reliably reach the control plane regardless of `allowedHostnames` configuration ## What's going on Local Paperclip agents were sometimes inheriting `PAPERCLIP_API_URL=http://192.168.1.50:3100` even when the server was bound to loopback, which made the control plane unreachable from this workspace. This keeps the runtime API on the loopback bind host for local startup while still preserving LAN candidates for other callers. ## Problem `choosePrimaryRuntimeApiUrl` preferred the first allowed hostname over the actual loopback bind host. In the failing setup from NEE-327, that exported `http://192.168.1.50:3100` into agent env even though `http://127.0.0.1:3100` was the reachable control-plane URL. ## Solution The primary runtime URL chooser now returns the normalized loopback bind host before considering `allowedHostnames`. I added a focused unit test for the chooser and a startup regression that verifies `PAPERCLIP_RUNTIME_API_URL` / `PAPERCLIP_API_URL` stay on `127.0.0.1` while the candidate list still includes the LAN hostname. ## Testing - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/runtime-api.test.ts src/__tests__/server-startup-feedback-export.test.ts` - Result: `2` files passed, `13` tests passed - Runtime check: `curl -sS http://127.0.0.1:3100/api/health` returned `{"status":"ok",...}` during this heartbeat ## Notes Already-running local Paperclip servers need a restart to export the corrected `PAPERCLIP_API_URL` into new agent runs. Rollback is revert `df8d7fcf`. ## Model Used Claude Sonnet 4.6 (`claude-sonnet-4-6`), 1M context window, via the Paperclip Founding Engineer agent harness. Capabilities used: extended tool use, code editing, test execution, shell commands. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [ ] 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: Neeraj Kumar Singh <b.nirajkumarsingh@hotmail.com> |
||
|
|
c0743482bc |
fix(claude-local): tolerate sandboxes whose Claude CLI lacks --effort (#8393)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The claude-local adapter launches the Claude CLI inside execution environments (local, SSH, and ephemeral sandboxes such as Daytona) > - Newer adapter code passes `--effort` to the CLI, but the Claude binary baked into some sandbox images is older and rejects it with `error: unknown option '--effort'`, so every run in those environments fails > - This needs addressing because the failure is environment-dependent and silent from the operator's perspective — the run just dies with a CLI usage error > - This pull request probes the in-sandbox CLI for `--effort` support once, caches the result per environment, and strips the flag (with a warning) when the CLI does not support it > - The benefit is that sandboxes with older Claude CLIs keep working instead of failing, with negligible probe overhead because the capability check is cached and reused across ephemeral leases ## Linked Issues or Issue Description No public GitHub issue exists. Describing the bug inline following the bug report template: ### What happened? Runs using the `claude-local` adapter inside certain sandbox/execution environments fail with `error: unknown option '--effort'`. The adapter unconditionally appends `--effort` to the Claude CLI invocation, but the Claude CLI version present in some sandbox base images predates that flag, so the process exits with a usage error and the run dies. ### Expected behavior The adapter should detect that the target environment's Claude CLI does not support `--effort` and degrade gracefully — drop the flag and emit a warning — rather than failing the run. ### Steps to reproduce 1. Configure an execution environment (e.g. a sandbox image) whose bundled Claude CLI is old enough to predate the `--effort` option. 2. Run any claude-local task that resolves to an effort level (so `--effort` is appended). 3. Observe the run fail immediately with `error: unknown option '--effort'`. ### Paperclip version or commit `master` at the time of this PR (branch forked from current `master`). ### Deployment mode Self-hosted / local instance using execution environments (reproducible with ephemeral sandbox providers such as Daytona where `reuseLease: false`). ### Agent adapter(s) involved Claude Code (`claude-local`). ## What Changed - Add a CLI capability probe (`cli-capabilities.ts`) that runs the target Claude binary's `--help` inside the execution environment to detect `--effort` support. - Strip `--effort` from the CLI args (emitting a warning) when the probe reports the flag is unsupported; keep it otherwise. - Cache probe results keyed by `sandbox:providerKey:environmentId:command` (no lease id) so the probe is reused across ephemeral leases — important for `reuseLease: false` sandbox configs like Daytona, which would otherwise re-probe on every run. - Conservative fallback: if the probe itself can't run/parse, assume the flag is supported (preserves prior behavior). ## Verification - `node_modules/.bin/vitest run src/__tests__/claude-local-execute.test.ts src/__tests__/claude-local-adapter-environment.test.ts` → **2 files, 30 tests passed**. - Regression test issues two `execute()` calls with distinct lease ids and asserts the in-sandbox `--help` probe runs exactly once (cache reuse across leases). - Added tests covering: flag stripped when unsupported, flag retained when supported, warning emitted, and conservative fallback when the probe fails. ## Risks Low risk. The change is additive and gated behind a probe with a conservative default (assume supported on probe failure), so existing environments that support `--effort` are unaffected. Worst case for an environment where the probe is unreliable is the prior behavior (flag passed through). ## Model Used Claude Opus 4.8 (claude-opus-4-8), extended thinking, with tool use / 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 (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 - [ ] If this change affects the UI, I have included before/after screenshots (N/A — no UI change) - [ ] I have updated relevant documentation to reflect my changes (N/A — no doc-facing behavior change) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending CI) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending review) - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
07e98d2b2c |
feat(adapter-utils): add observable sandbox sync progress (#8395)
## Thinking Path > - Paperclip is the control plane for running AI-agent companies, so long-running remote work needs to stay observable to human operators. > - Cloud / sandbox agents are an active roadmap area, and their workspace sync path is part of the runtime substrate every remote coding run depends on. > - In the sandbox and SSH execution-target flows, Paperclip logged that sync had started, then often went silent for the full transfer window. > - That made large remote syncs feel stalled and also hid a real performance problem in the command-managed sandbox upload path. > - The first part of this pull request threads a throttled progress-reporting surface through the adapter execution-target stack so sync and restore work can emit meaningful updates. > - The second part fixes the command-managed sandbox transport itself: it removes the old serial 32KB append bottleneck, but also falls back away from the single-stream path when a provider-backed sandbox runner cannot surface mid-flight stdin progress. > - The result is that sandbox and SSH transfers are both faster and more observable, including the live Daytona-style sandbox case that previously only emitted `0%` and `100%`. ## Linked Issues or Issue Description No public GitHub issue exists for this bug, so it is described inline below following the bug report template. ### What happened - Remote sandbox and SSH workspace syncs could spend a long time transferring data while only logging a start line (`Syncing workspace and runtime assets to sandbox environment`) and, at best, a terminal line. - In the command-managed sandbox path, the original upload implementation also paid a large performance cost by appending base64 data in many small sequential remote writes (thousands of serial 32KB round-trips on a large workspace). - After the initial transport rewrite, live provider-backed sandbox runs still only emitted `0%` and `100%` because the single-stream stdin RPC buffered progress until completion. ### Expected behavior - Long-running sandbox and SSH syncs should periodically report how much of the transfer is complete (a percentage and/or MB transferred) so an operator can tell the run is healthy and making progress rather than stuck. - The main sandbox upload path should not be artificially slow. - A transfer that fails partway should leave an explicit failure marker in the log rather than a dangling intermediate percentage. ### Steps to reproduce 1. Run an agent against a sandbox (command-managed) or SSH (remote-managed) execution target with a non-trivial workspace. 2. Watch the run log during the workspace/runtime asset sync phase. 3. Observe that the log shows the sync start line and then stays silent for the full transfer (live provider-backed sandbox runs only show `0%` then `100%`). ### Paperclip version or commit - Branch `PAPA-825-provide-status-updates-when-syncing-sandboxes` off `master`. ### Deployment mode - Self-hosted / local instance using sandbox (command-managed) and SSH (remote-managed) execution targets, including provider-backed sandbox runners. ## What Changed - Added shared throttled runtime progress reporting and threaded `onProgress` through the adapter execution-target surface and adapter `execute.ts` entrypoints. - Added sync and restore progress reporting for the command-managed sandbox path and the SSH/remote-managed path, including git import/export progress where totals are known. - Reworked command-managed sandbox transfer behavior so uploads use the faster single-stream path when appropriate, but fall back to chunked progress-emitting writes when the runner cannot expose mid-stream stdin progress. - Marked provider-backed environment sandbox runners as not supporting single-stream stdin progress so live sandbox runs emit meaningful intermediate updates instead of only `0%` and `100%`. - Emit an explicit terminal failure marker (`failed at NN% (x/y MB)`) when an SSH/tar transfer rejects, so a failed sync no longer leaves a dangling intermediate percentage in the log. - Run the SSH sync/restore size estimate (local directory walk / remote `du` probe) concurrently with the transfer instead of awaiting it before opening the pipe, so progress instrumentation no longer adds startup latency proportional to workspace file count. - Added and extended focused regression coverage for runtime progress throttling and the new failure marker, command-managed sandbox transfers, sandbox orchestration, SSH transfer progress, and environment execution-target wiring. ## Verification - `pnpm exec vitest run packages/adapter-utils/src/runtime-progress.test.ts packages/adapter-utils/src/ssh-fixture.test.ts packages/adapter-utils/src/command-managed-runtime.test.ts packages/adapter-utils/src/sandbox-managed-runtime.test.ts` - `pnpm exec vitest run packages/adapter-utils/src/command-managed-runtime.test.ts server/src/__tests__/environment-execution-target.test.ts` - `npx tsc --noEmit` for `packages/adapter-utils` ## Risks - The provider-backed sandbox fallback now prefers chunked command-managed writes when progress hooks are active, so small-to-medium uploads may trade some raw throughput for observable intermediate progress on runtimes that cannot surface true mid-stream stdin progress. - Progress percentages on tar-based transfers still depend on estimates in some cases, so operators may briefly see MB-only lines before the estimate resolves, then near-final clamping before the terminal `100%` line. - This PR changes shared execution-target behavior used by multiple adapters, so regressions would most likely appear in remote runtime setup/teardown flows rather than in a single adapter. ## Model Used - Initial implementation: OpenAI GPT-5.4 via Codex local agent (`codex_local`), high reasoning mode. - Observability follow-ups (failure marker, concurrent size estimate, added tests): Claude Opus 4.8 via Claude Code (`claude_local`). ## 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 - [ ] 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 - [ ] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
5cace19aca |
Remove adapter support matrix table from Company Environments (#8398)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run in execution environments, and the Company Environments settings page lets operators configure them > - That page rendered an "environment support by adapter" matrix table plus two descriptive text boxes explaining the adapter/driver/sandbox support model > - That matrix duplicated information already surfaced where operators actually pick a driver/provider, and added a wide, dense table that provided no actionable value on this screen > - This pull request removes the support-matrix table and the two explanatory text boxes above it, along with the now-orphaned helper component, derived constant, and unused imports > - The benefit is a simpler, less cluttered environments settings page that only shows the controls an operator acts on ## Linked Issues or Issue Description No public GitHub issue exists for this. Describing the change following the feature-request template: ### Problem or motivation The Company Environments settings page showed an adapter support matrix table (Adapter × Local/SSH/Sandbox) and two paragraphs of descriptive text above it. The table restated the static adapter/driver support model and the sandbox-provider plugin caveat, neither of which is actionable on this page — operators don't change adapter capabilities here, they create and edit environments. The table was wide enough to require horizontal scroll and added visual noise without helping the operator complete any task. ### Proposed solution Remove the support-matrix table and the two descriptive text boxes above it from `CompanyEnvironments.tsx`, along with the now-unused helper component, derived constant, locals, and imports. Leave all environment create/edit controls, provider selection, and sandbox-provider logic untouched. ### Alternatives considered Collapsing the table behind a disclosure/"Learn more" toggle instead of removing it — rejected because the information is static, non-actionable on this screen, and already available where operators pick a driver/provider; hiding it would keep the maintenance cost without adding value. ### Roadmap alignment Not a roadmap feature — this is a small, focused UI cleanup that removes non-actionable content from an existing settings page. Related (already merged) work that last touched this copy: paperclipai/paperclip#4902 (clarified sandbox-provider messaging in company environments). No open duplicate PR was found. ## What Changed - Removed the adapter support matrix `<table>` from `ui/src/pages/CompanyEnvironments.tsx` - Removed the two descriptive text boxes rendered above that table (adapter support model + installed sandbox providers blurbs) - Deleted the now-unused `SupportMark` helper component and the `ENVIRONMENT_SUPPORT_ROWS` derived constant - Removed the now-unused `sandboxSupportVisible` local and four now-unused imports (`AGENT_ADAPTER_TYPES`, `getAdapterEnvironmentSupport`, `Check`, `adapterLabels`) - Updated `ui/src/pages/CompanySettings.test.tsx` to drop the two assertions tied to the removed copy ## Verification - `vitest run src/pages/CompanySettings.test.tsx` → 4/4 pass - `tsc --noEmit` on the UI project → no errors in the changed files - The remaining environment create/edit controls, provider selection, and sandbox-provider logic are untouched (the `environmentCapabilities`, `discoveredPluginSandboxProviders`, and `sandboxCreationEnabled` values are still used by the form) ## Risks Low risk — pure presentational removal of a read-only informational table and static copy. No API, data model, or behavioral changes; no state that other components depend on was removed. ## Model Used Claude (Anthropic), model id `claude-opus-4-8` (Claude Opus 4.x family), with extended thinking and tool use, run 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 - [ ] 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 - [ ] 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: Paperclip <noreply@paperclip.ing> |
||
|
|
e9931f116a |
fix(ui): move environment create/edit into a dialog (#8391)
## Thinking Path > - Paperclip is the open source control plane people use to manage AI agents and the work they do. > - This change lives in the board UI, specifically the instance environments settings page where operators define reusable execution targets. > - That page currently rendered the create/edit environment form inline near the bottom of the screen. > - Because the form appeared away from the action that triggered it, it was easy to miss and felt inconsistent with the rest of the UI. > - This pull request moves environment creation and editing into a proper centered dialog with the existing form behavior preserved. > - The benefit is a more obvious, consistent, and easier-to-complete settings flow for environment management. ## Linked Issues or Issue Description No public GitHub issue matched this exact UI problem when I searched related issues and PRs. ### Pre-submission checklist - [x] I have searched existing open and closed issues and this is not a duplicate. - [x] I am on the latest released version of Paperclip (or can reproduce on `master`). - [x] I have confirmed the issue originates in Paperclip itself, not adapter or local configuration. ### What happened? On `Settings -> Instance settings -> Environments`, clicking create or edit revealed the environment form inline near the bottom of the page. Because the form appeared away from the action that triggered it, it was easy to miss. ### Expected behavior Create and edit should open a centered modal dialog with a dimmed backdrop and the same form controls. ### Steps to reproduce 1. Open `Settings -> Instance settings -> Environments`. 2. Click the create or edit action for an environment. 3. Observe that the form expands inline on the page instead of opening in a modal. ### Paperclip version or commit Observed on current `master` before this PR, for example [`e93d78b46`](https://github.com/paperclipai/paperclip/commit/e93d78b46). ### Deployment mode Local dev (`pnpm dev`). ### Installation method Built from source (`pnpm dev` / `pnpm build`). ### Agent adapter(s) involved - [x] Not adapter-specific (core bug) ### Database mode Embedded PGlite (default, `DATABASE_URL` unset). ### Access context Board (human operator). ### Node.js version Node.js 25.6.1 in the local contributor environment. ### Operating system macOS local development environment. ### Relevant logs or output None. This was a visible UI behavior issue rather than a logged server/runtime error. ### Relevant config (if applicable) Not config-related. ### Additional context Related PR/search context checked before opening this fix. UI preview screenshots are attached below. ### Privacy checklist - [x] I have reviewed all pasted output for PII and redacted where necessary. ## What Changed - Moved the environment create/edit form in `ui/src/pages/CompanyEnvironments.tsx` into a shared dialog. - Added an explicit `Add environment` action near the saved environments list and wired row edit actions to open the same dialog in prefilled mode. - Preserved existing save/test behavior while resetting dialog-local mutation state when the modal opens or closes. - Extended `ui/src/pages/CompanyEnvironments.test.tsx` to cover add-open/cancel and edit-open/save dialog flows. - Updated `ui/src/pages/CompanySettings.test.tsx` so the existing environments coverage opens and inspects the dialog through the Radix portal. ## Verification - `node node_modules/vitest/vitest.mjs run ui/src/pages/CompanyEnvironments.test.tsx` - `node node_modules/vitest/vitest.mjs run ui/src/pages/CompanySettings.test.tsx` - Manual review: - Open `Settings -> Instance settings -> Environments` - Click `Add environment` and confirm the form opens in a centered dialog - Click `Edit` on an existing environment and confirm the dialog opens with existing values - Confirm `Cancel`, `Test`, and save actions still behave as expected - Before/after screenshots: - [Cancel environment creation/edit](https://artifacts.cutter.sh/8391/run-1175b5c-2026-06-20T18-47-11/preview/clip-01.mp4) - [Add a new environment](https://artifacts.cutter.sh/8391/run-1175b5c-2026-06-20T18-47-11/preview/change-02.png) - [Save an environment](https://artifacts.cutter.sh/8391/run-1175b5c-2026-06-20T18-47-11/preview/change-03.png) ## Risks - Low risk overall because the change is contained to the environments page and keeps the existing form fields and mutations. - The main behavior change is modal layout, so the biggest risk is regressions in tall-form scrolling or smaller-screen dialog ergonomics. > I checked [`ROADMAP.md`](ROADMAP.md). This is a tightly scoped UI polish fix, not duplicate roadmap-level core feature work. ## Model Used - OpenAI GPT-5.4 via the `codex_local` Paperclip adapter - High reasoning mode with tool use, shell execution, git operations, and targeted test execution - Model-assisted authoring and verification of the UI and test changes ## 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] If this change affects the UI, I have included before/after screenshots - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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: Paperclip <noreply@paperclip.ing> |
||
|
|
e93d78b46c |
fix(server): harden live events upgrade sockets (#8383)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server exposes live event updates to board and agent clients over WebSocket upgrade requests > - WebSocket upgrade authorization can be asynchronous, leaving the raw HTTP upgrade socket in server-owned code before `ws` takes over > - If the client disconnects during that authorization window, the server can still try to reject or upgrade a closed socket > - A raw socket write after peer disconnect can emit `EPIPE` / `ECONNRESET`, and without a listener that can become process-fatal > - This pull request hardens the pre-`ws` upgrade socket path and adds regression coverage for disconnect/error races > - The benefit is that live event reconnect churn degrades gracefully instead of risking a server crash ## Linked Issues or Issue Description External public context: Refs https://github.com/aronprins/paperclip-desktop/issues/14 No matching open upstream issue or PR was found when searching `paperclipai/paperclip` for `EPIPE`, `rejectUpgrade`, `live-events-ws`, `websocket upgrade`, and related socket-write terms. ### Bug report **What happened?** The live events WebSocket upgrade handler could write a rejection response to the raw upgrade socket after the peer had already disconnected during async authorization. A transport-level `EPIPE` or similar socket error on that raw socket can terminate the server process if it is emitted without an error listener. **Expected behavior** The server should not write to destroyed/non-writable upgrade sockets, should tolerate raw socket errors while authorization is pending, and should not call `handleUpgrade()` after the socket is no longer writable. **Steps to reproduce** 1. Start a Paperclip server with live events enabled. 2. Open a raw WebSocket upgrade request to `/api/companies/:companyId/events/ws`. 3. Disconnect the client before async authorization resolves. 4. Let the server take the reject or upgrade path. 5. Observe that the pre-fix path can still write to or upgrade a closed socket. **Paperclip version or commit** Reproduced by static inspection on `master` before this PR at `950484d20`. **Deployment mode** Any mode using live event WebSocket upgrades. The report was originally observed from local Desktop embedding, but the vulnerable code is in the server package. ## What Changed - Added a writable-state guard for raw upgrade sockets before rejecting or completing an upgrade. - Changed rejection responses from raw `write()` + `destroy()` to guarded `end()` with warning logging if rejection fails synchronously. - Attached a temporary raw socket error listener during the async upgrade authorization window and cleaned it up on socket close or successful `ws.handleUpgrade()`. - Added regression tests for rejecting after an early socket close and for handling raw socket `error` events while authorization is pending. ## Verification - `pnpm exec vitest run server/src/__tests__/live-events-ws.test.ts` - `pnpm --filter @paperclipai/server typecheck` - `git diff --check` ## Risks - Low risk: the change is scoped to the live-events WebSocket upgrade path before `ws` takes ownership of the socket. - Rejected upgrade responses now use graceful `socket.end(...)`; clients should still receive the same HTTP status text when the socket is writable. - The temporary error listener is removed before successful `handleUpgrade()` so normal WebSocket client error handling remains owned by the existing `connection` path. > 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 coding agent with repository-aware tool use, shell command execution, GitHub connector access, and local code editing. ## 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] 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 - [ ] 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: Paperclip <noreply@paperclip.ing> |
||
|
|
fce3b439af |
fix: warn operators that experimental features may break (#8382)
## Thinking Path > - Paperclip is the control plane operators use to manage AI-agent companies. > - Board operators rely on the settings UI and CLI docs to understand which product surfaces are stable to depend on. > - Experimental features already existed in the product, but the operator-facing contract around them was too soft and too fragmented. > - That created a risk that users would enable experiments without being told clearly that they can break, change, or disappear. > - The docs and the in-product settings page both needed the same explicit warning language so the contract is visible at the moment of decision. > - This pull request adds that warning to the board-operator guide, CLI references, and the experimental settings page. > - The benefit is clearer operator expectations without changing the underlying feature flags or rollout behavior. ## Linked Issues or Issue Description No public GitHub issue exists for this docs/polish gap. Problem description: - Board operators could enable experimental features without a clear operator-facing statement that those features are opt-in and come without compatibility guarantees. - The docs site, repo CLI reference, and in-product experimental settings page did not present one consistent warning contract. - This PR closes that gap by documenting the risk explicitly where operators discover and enable those settings. Related public search: - Searched public issues/PRs for related work with `gh search issues --repo paperclipai/paperclip 'experimental features warning'` and `gh search prs --repo paperclipai/paperclip 'experimental features warning'`. - Reviewed open PR #6165 during that search and found it unrelated; it changes experimental auth/routing flags rather than documenting experimental-feature risk. ## What Changed - Added a new board-operator guide at `docs/guides/board-operator/experimental-features.md` that defines the Paperclip contract for experimental features. - Registered that guide in `docs/docs.json` so it appears in the public docs navigation. - Added matching caveat language next to `instance settings:experimental` in `docs/cli/control-plane-commands.md`. - Added the same caveat to `doc/CLI.md` so the repo CLI reference does not drift from the published docs. - Added a single page-level warning banner to `ui/src/pages/InstanceExperimentalSettings.tsx` stating that experimental features are opt-in, carry no compatibility guarantees, and may change, break, or be removed. - Added a targeted UI test in `ui/src/pages/InstanceExperimentalSettings.test.tsx` that asserts exactly one page-level warning renders with the new risk language. ## Verification - `jq empty docs/docs.json` - `git diff --check` - `cd ui && pnpm vitest run src/pages/InstanceExperimentalSettings.test.tsx` - Manual review of the warning contract across: - `docs/guides/board-operator/experimental-features.md` - `docs/cli/control-plane-commands.md` - `doc/CLI.md` - `ui/src/pages/InstanceExperimentalSettings.tsx` UI note: - This is a copy-level warning addition rather than a layout rework. I did not attach before/after screenshots in this PR body. ## Risks - Low risk: this changes operator-facing documentation and warning copy, not feature-flag behavior. - The main failure mode is wording drift across docs and UI in future edits, which is why this PR adds the same contract to all relevant operator-facing surfaces. > I checked `ROADMAP.md` before opening this PR. This is docs/UI polish around an existing experimental surface, not overlapping roadmap-level core feature work. ## Model Used - OpenAI Codex Local using `gpt-5.4` with high reasoning and tool use for coordination, review, docs changes, and PR preparation. - Anthropic Claude Local using `claude-opus-4-8` with high reasoning and tool use for the in-product warning and targeted UI test. ## 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 - [ ] 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 - [ ] 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: Paperclip <noreply@paperclip.ing> |
||
|
|
547463d3a2 |
refactor(environments): make execution environments instance-scoped (#8375)
## Thinking Path > - Paperclip is the control plane for AI-agent companies, so execution environment selection has to stay inspectable and predictable across companies, agents, and runs. > - The environment subsystem decides where an agent heartbeat actually runs and how remote sandbox state is realized and restored. > - That subsystem previously mixed company-scoped environment catalogs with issue-level environment stamping, so a reassigned issue could keep executing in the previous assignee's sandbox. > - That behavior breaks the control-plane contract: changing the assignee should change the executing agent/environment path unless there is an explicit current override. > - Fixing it cleanly required more than a narrow patch; the environment model had to move to instance scope with a single inherited default and per-agent override semantics. > - This pull request rewires the schema, server/API surface, runtime resolution, and UI around that model, then adds regression coverage for cross-company inheritance and per-agent isolation. > - The benefit is that environment choice now follows the approved instance/agent configuration path instead of stale issue state, while shared environments only need to be configured once per instance. ## Linked Issues or Issue Description - No directly matching public GitHub issue or PR was found while searching for this refactor. ### What happened? Reassigning work between agents with different execution environments could keep running in the previous sandbox because environment choice was stamped onto the issue and outranked the current assignee. The same subsystem also forced environment catalogs to be duplicated per company even though the underlying execution environments were instance-wide resources. ### Expected behavior Execution should resolve through the current instance and agent configuration path, with one instance-scoped environment catalog, one instance default, optional per-agent override, and no stale issue-level environment authority surviving reassignment. ### Steps to reproduce 1. Configure two agents to use different execution environments. 2. Assign an issue to the first agent so the issue records execution state in that environment. 3. Reassign the same issue to the second agent and run another heartbeat. 4. Observe that the pre-fix runtime can still sync or execute in the original sandbox instead of the second agent's environment. ### Paperclip version or commit Current `master` before this PR. ### Deployment mode Self-hosted server. ### Installation method Built from source (`pnpm dev` / `pnpm build`). ### Agent adapter(s) involved - Claude Code - Not adapter-specific (core bug in environment authority / resolution) ### Database mode External Postgres. ### Access context Both board reassignment and agent heartbeats were involved. ## What Changed - Moved environments and their default selection contract to instance scope in DB/shared types, including the migration that dedupes legacy per-company environments and seeds the instance local default. - Reworked environment CRUD/auth flows to use instance-scoped APIs and added route/service coverage for instance-level environment management. - Changed runtime resolution to prefer `agent default -> instance default -> built-in local`, removed issue-level environment stamping from the active execution path, and isolated sandbox/plugin leases by `(executionWorkspaceId, agentId)`. - Added environment env-var runtime precedence so environment-provided values act as the baseline for agent execution. - Moved the environment UI into instance settings and updated agent configuration surfaces to reflect inherit/override behavior. - Added regression coverage for instance-default inheritance across companies and for the new runtime resolution behavior. - Fixed a rebase-only duplicate `enableTaskWatchdogs` flag regression in instance settings types/validators/services so the branch typechecks cleanly on current `master`. - Updated stale server tests so CI matches the shipped instance-scoped environment contract. ## Verification - `git diff --check` - `pnpm --filter @paperclipai/shared typecheck` - `pnpm --filter @paperclipai/db typecheck` - `pnpm exec vitest run server/src/__tests__/environment-runtime-driver-contract.test.ts server/src/__tests__/agent-permissions-routes.test.ts server/src/__tests__/environment-routes.test.ts server/src/__tests__/environment-instance-routes.test.ts server/src/__tests__/execution-workspace-policy.test.ts server/src/__tests__/heartbeat-plugin-environment.test.ts server/src/__tests__/instance-settings-routes.test.ts` ## Risks - The migration changes environment scope and dedupes existing rows, so installs with unusual legacy environment combinations should be reviewed carefully during upgrade. - Remote execution behavior now depends on instance-default inheritance semantics instead of issue-level stamping, so any remaining code paths that still assume issue-scoped environment authority would surface as follow-up bugs. - This PR includes both server/runtime behavior and UI relocation, so reviewers should watch for authorization edge cases around instance settings and environment management. > I checked [`ROADMAP.md`](ROADMAP.md). This work fits the existing Cloud / Sandbox agents direction as a bug-fix/refactor to current behavior, not a new parallel product surface. ## Model Used - OpenAI Codex coding agent in this Paperclip/Codex session; GPT-5-class tool-using model with code execution and shell access. The exact backend model ID is not exposed to the session runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have 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 - [ ] 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 - [ ] 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 |
||
|
|
950484d204 |
fix: scope environments "Test provider" button to clicked row (#8380)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run inside environments, and the Environments settings page lets users configure each environment and verify it with a "Test provider" / "Test connection" button per row > - When a user clicked one environment's test button, every environment row's button switched to the disabled "Testing..." state at the same time, then all flipped back together > - That happened because all rows read the same shared `environmentProbeMutation.isPending` flag, so a single in-flight probe disabled and relabeled every button > - This is confusing: it looks like every environment is being tested, and it blocks interacting with other rows while one probe runs > - This pull request tracks the specific environment id being probed in dedicated state and scopes the disabled/label logic to that id > - The benefit is that only the button the user actually clicked shows "Testing..." and is disabled, while the other rows stay interactive ## Linked Issues or Issue Description No public GitHub issue exists, so the bug is described inline below following the bug report template. ### What happened? On the Environments settings page, clicking "Test provider" on one environment caused the test button on *every* environment row to change to "Testing..." and become disabled at the same time, then all reverted together when the probe finished. ### Expected behavior Only the button for the environment the user clicked should show "Testing..." and be disabled while its probe runs. Every other row's button should stay enabled and unchanged. ### Steps to reproduce 1. Open instance settings → Environments with two or more configured environments. 2. Click "Test provider" / "Test connection" on a single row. 3. Observe that all rows' buttons enter the "Testing..." disabled state simultaneously instead of just the clicked one. ### Paperclip version or commit `master` at commit a10f17800 (branch `fix/environments-test-provider-button-scope`). ### Deployment mode Local dev (`pnpm dev`). UI-only; reproduces independent of backend. ## What Changed - Added a dedicated `testingEnvironmentId` state in `ui/src/pages/CompanyEnvironments.tsx` to track which environment is currently being probed. - Set it in the probe mutation's `onMutate` and clear it in `onSettled`, and reset it when the selected company changes. - Scoped the test button's `disabled` state and `"Testing..."` label to `testingEnvironmentId === environment.id` instead of the shared `environmentProbeMutation.isPending` flag. - Added `ui/src/pages/CompanyEnvironments.test.tsx` verifying that clicking one environment's test button puts only that row into the "Testing..." disabled state while other rows stay enabled. ## Verification - `pnpm typecheck` (UI) passes clean. - `vitest run src/pages/CompanyEnvironments.test.tsx` passes (new test fails against the old shared-`isPending` behavior, confirming it guards the fix). - Manual: on the Environments page with multiple environments, click one row's test button and confirm only that button shows "Testing..." / is disabled while the others remain enabled, then it reverts on completion. ## Risks - Low risk. Single-file UI change scoped to per-row button state; no API, schema, or behavior changes to the probe itself. The probe mutation still runs identically — only which buttons reflect the pending state changed. ## Model Used - Claude Opus 4.8 (Anthropic), `claude-opus-4-8`, with extended reasoning and tool use, 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 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 - [ ] If this change affects the UI, I have included before/after screenshots - [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 |
||
|
|
3b9f36403e |
refactor(ui): vertically center task status circle in task list (#8376)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The web UI renders work items in a task/issue list and board columns, each row leading with a status circle next to the title text > - The status-circle wrapper `<span>` was not enforcing cross-axis centering, so the circle aligned to the top of taller rows instead of the title > - This looked broken: the circle floated above the text rather than sitting centered against it, across statuses > - This pull request adds flex centering (`inline-flex` + `items-center`) to the status-icon wrapper spans so the circle is vertically centered with the title in every row > - The benefit is a consistent, correctly aligned task list regardless of task status or row height ## Linked Issues or Issue Description No public GitHub issue exists; describing inline following the bug report template. ### What happened? In the task/issue list and board, the leading status circle was not vertically centered with the task title text. On taller rows the circle sat near the top instead of aligned with the title, so the list looked misaligned across all statuses. ### Expected behavior The status circle should be vertically centered with the title string in every task row, regardless of status. ### Steps to reproduce 1. Open the task list (or board) view in the web UI. 2. Observe rows in different statuses, especially taller rows. 3. Notice the status circle is top-aligned rather than vertically centered with the title text. ### Paperclip version or commit `master` @ 411a9a51 (branch base for this PR). ### Deployment mode Local dev (pnpm dev). ## What Changed - `ui/src/components/IssueColumns.tsx`: added `items-center` to the leading status-icon wrapper span (`inline-flex` container) in `InboxIssueMetaLeading`. - `ui/src/components/IssuesList.tsx`: added `inline-flex items-center` to the two status-icon wrapper spans so the circle centers with the row text (matching the `IssueColumns.tsx` change). ## Verification - Manual: open the task list and board views; confirm the status circle is vertically centered with the title across statuses and on taller rows. - Before/after matches the screenshots from the originating request (broken: circle top-aligned; fixed: circle centered). - `cd ui && npx tsc --noEmit` — type check passes for the touched components. ## Risks - Low risk. CSS-only change to wrapper spans; no logic, data, or API changes. Limited to status-icon alignment in the task list/board rows. ## Model Used - Claude Opus 4.6 (`claude-opus-4-6`), extended thinking + tool use, 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 and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [ ] I have added or updated tests where applicable (CSS-only alignment change; no meaningful unit test — retitled `refactor:` per contribution gate) - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes (none required) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
eb68198c42 |
feat(adapter-claude-local): emit errorCode claude_refusal on stop_reason refusal (#8314)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run through adapters; `@paperclipai/adapter-claude-local` shells out to the Claude CLI and maps its result JSON into Paperclip's run outcome (`errorCode` / `errorFamily`) > - A Fable 5 policy refusal exits the CLI cleanly (`exitCode=0`, `is_error=false`) with `stop_reason: "refusal"`, which the adapter mapped to `errorCode: null` > - Paperclip therefore recorded the run as a silent success, so the agent's heartbeat stalled with no signal for operators and no hook to retry or alert > - This pull request adds a refusal detector to the adapter so a refusal resolves to a distinct `errorCode: "claude_refusal"` (`errorFamily: "model_refusal"`) > - The benefit is that policy refusals become observable: the server persists the code, operators can see it on the run, and downstream retry/alert policy can key off it ## Linked Issues or Issue Description No public GitHub issue exists. Describing the underlying bug in-PR, following `.github/ISSUE_TEMPLATE/bug_report.yml`: **What happened?** When the Claude CLI returns `stop_reason: "refusal"` (a model policy refusal), it still exits cleanly (`exitCode=0`, `is_error=false`). `@paperclipai/adapter-claude-local` keyed refusal detection off the failure flag during error-code resolution, so it returned `errorCode: null` — a refused run was indistinguishable from a successful one, recorded by Paperclip as a silent success with no signal to retry or alert. **Expected behavior** A refusal should resolve to a distinct, non-null error code (`errorCode: "claude_refusal"`, `errorFamily: "model_refusal"`) so the server can surface it to operators and downstream policy can react. **Steps to reproduce** 1. Run any agent on the `claude-local` adapter with a prompt the model refuses on policy grounds. 2. The Claude CLI exits `0` with `is_error=false` and `stop_reason: "refusal"`. 3. Observe the run resolves `errorCode: null` (pre-fix) instead of a refusal-specific code. **Paperclip version or commit** `adapter-claude-local` v2026.609.0 (`dist/server/execute.js` error-code resolution block); reproduced on `master`. **Deployment mode** Local / self-hosted. **Agent adapter(s) involved** `@paperclipai/adapter-claude-local` (Claude Code, local). ## What Changed - **`packages/adapters/claude-local/src/server/parse.ts`** — new `isClaudeRefusalResult()` helper, parallel to `isClaudeMaxTurnsResult()`. Detects `stop_reason` / `stopReason` / `error_code` / `errorCode` == `refusal` and `subtype` == `model_refusal`, case/whitespace tolerant. - **`packages/adapters/claude-local/src/server/execute.ts`** — compute the refusal flag independent of the `failed` flag (a refusal exits cleanly, so keying off `failed` would miss it); resolve `errorCode: "claude_refusal"` and `errorFamily: "model_refusal"`; surface `stopReason: "refusal"` in `resultJson`. - **`packages/adapter-utils/src/types.ts`** — widen `AdapterExecutionErrorFamily` with `"model_refusal"`. - **`packages/adapters/claude-local/src/server/index.ts`** — export the new helper. - **`packages/adapters/claude-local/src/server/parse.test.ts`** — 7 new unit tests. ## Verification ``` npx vitest run packages/adapters/claude-local # 35 passed (7 new) npx tsc --noEmit -p packages/adapter-utils # clean npx tsc --noEmit -p packages/adapters/claude-local # clean ``` Manual: a result JSON with `stop_reason: "refusal"` on a clean exit now resolves `errorCode: "claude_refusal"` and `errorFamily: "model_refusal"`; non-refusal results are unaffected. ## Risks Low risk. Additive only — it introduces a new error code on a path that previously returned `null`; no existing error code or success path changes. `claude_refusal` is deliberately **not** added to the transient-upstream retry set: a refusal is deterministic, so retrying the same prompt yields the same refusal. It surfaces as a distinct non-transient code for operators; downstream retry/alert policy can key off `claude_refusal` / `errorFamily: model_refusal` later. ## Model Used Claude Opus 4.8 (`claude-opus-4-8`), extended-thinking mode, run via Claude Code 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 (no related PRs 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] 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 (N/A — no UI changes) - [x] I have updated relevant documentation to reflect my changes (N/A — internal adapter error-code addition; no user-facing docs) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (this push re-runs the gates; will confirm once green) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (prior run was 4/5 on a PR-description note now addressed; will confirm on re-run) - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Full-Stack Engineer <noreply@paperclip.ing> |
||
|
|
631b7806ed |
Add heartbeat preflight budget caps (#8347)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Heartbeats are the control-plane path that turns scheduled,
comment-driven, or on-demand wakeups into adapter executions.
> - Budgeting and recurring work need enforcement before an adapter
starts, not only after model usage is recorded.
> - Empty timer wakes also need an opt-in fast-exit path so operators
can keep routine schedules without paying for no-op model turns.
> - This pull request adds heartbeat preflight gates for daily run and
daily cost caps, plus an explicit timer no-work skip policy.
> - The benefit is safer autonomous operation: capped agents stop before
new execution, queued work is cancelled cleanly at claim time, and
proactive agents still run by default unless the operator opts into
no-work skipping.
## Linked Issues or Issue Description
No public issue exists for this change. Inline bug report:
### What happened?
Heartbeat execution can start without enforcing per-agent daily
invocation and spend limits at the heartbeat boundary. A run that was
queued before a cap was reached can also be claimed later and invoke the
adapter unless the cap is checked again immediately before execution.
Operators also do not have an explicit opt-in fast-exit policy for
generic timer wakes with no actionable assigned work.
### Expected behavior
Configured daily run and daily cost caps should stop new heartbeat runs
before adapter execution. Already queued runs should be rechecked at
claim time and cancelled cleanly when a cap is now reached. Queued issue
runs cancelled by daily caps should release their issue execution locks
and promote deferred wakeups without entering immediate recovery loops.
Generic timer no-work skipping should be opt-in so proactive agents
continue to run by default.
### Steps to reproduce
1. Configure an agent heartbeat policy with a one-run daily cap or a
daily cost cap.
2. Create or queue heartbeat wakeups for that agent after the cap has
already been consumed.
3. Observe that without preflight and claim-time checks, the heartbeat
path can still enqueue or claim work that should be blocked before
adapter execution.
### Paperclip version or commit
Reproduced against `master` before this branch.
### Deployment mode
Local dev (`pnpm dev`) / built from source.
### Installation method
Built from source (`pnpm dev` / `pnpm build`).
### Agent adapter(s) involved
Not adapter-specific (core heartbeat scheduling and claim logic).
### Database mode
External Postgres in tests via embedded test harness.
### Access context
Not applicable.
### Node.js version
Node 20 in CI-compatible local development.
### Operating system
macOS local development, Linux CI-compatible tests.
### Relevant logs or output
The regression suite added in this PR covers the failing paths:
```shell
pnpm exec vitest run server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts
```
### Relevant config (if applicable)
```json
{
"heartbeat": {
"maxDailyRuns": 1,
"maxDailyCostCents": 1,
"skipTimerWhenNoActionableWork": true
}
}
```
### Additional context
This affects recurring/autonomous operation because the safest place to
stop excess work is before adapter execution starts.
### Privacy checklist
Reviewed for sensitive data; no private logs, credentials, or local
instance URLs are included.
## What Changed
- Added heartbeat policy parsing for per-agent daily run caps, daily
cost caps, and opt-in no-actionable-work timer skipping.
- Added pre-queue daily cap checks while preserving same-issue wake
coalescing.
- Added claim-time cap checks so already queued runs are cancelled
before adapter execution when a cap is reached.
- Added skipped wakeup metadata for cap and timer fast-exit decisions.
- Released issue execution locks for queued issue runs cancelled by
daily caps, with deferred wake promotion and without immediate recovery
loops while caps are active.
- Added regression coverage for timer skipping, proactive default
behavior, run caps, cost caps, queued-run cancellation, started
cancelled runs, and deferred issue wake promotion.
## Verification
- `git diff --check`
- `node -c server/src/services/heartbeat.ts`
- `pnpm exec vitest run
server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts`
- `pnpm --filter @paperclipai/server typecheck`
- Local autoreview: `skills/autoreview/scripts/autoreview --mode branch
--base origin/master --engine codex --model gpt-5.5 --thinking high`
- Result: clean, no accepted/actionable findings
## Risks
- Medium operational risk because this changes heartbeat scheduling and
claim-time behavior.
- The no-actionable-work timer fast-exit is explicitly opt-in to avoid
suppressing proactive agents unexpectedly.
- Daily run caps count runs by `startedAt` so old queued rows do not
consume today’s cap, while started runs still count even if they later
end as cancelled.
- Queued issue-run cap cancellation uses the existing release/promotion
path with immediate recovery suppressed to avoid retry loops while caps
are active.
## Model Used
Codex with GPT-5.5 high reasoning assisted with implementation, local
testing, and autoreview. The final review gate used local autoreview
with `gpt-5.5` high 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)
- [ ] 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] 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
- [ ] 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
|
||
|
|
323b6220cc |
Fix Gemini headless invocation stalls (#8368)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Local adapters turn Paperclip runs into unattended CLI invocations. > - The `gemini_local` adapter depends on Gemini CLI behaving non-interactively in headless worker sessions. > - When Gemini CLI falls back to browser-based auth, the process can stall at startup instead of producing stream-json output. > - The adapter should make headless intent explicit and turn missing auth into a classified, fast failure. > - This pull request hardens the Gemini child-process environment and parser classification around that failure mode. > - The benefit is that operators get actionable `gemini_auth_required` failures instead of silent hung runs. ## Linked Issues or Issue Description No exact public issue exists for this runtime stall. Related: #2344 covers a Gemini CLI adapter environment/auth probe failure. This PR addresses a different runtime path: unattended `gemini_local` executions that can stall when Gemini CLI attempts interactive browser auth in a headless session. Bug summary: - Adapter: `gemini_local` - Symptom: child Gemini CLI process starts but produces no stream-json output when auth requires interactive browser flow - Expected: unattended runs either produce stream-json output or fail quickly with a classified auth error - Actual: the run can hang at invocation until an external watchdog kills it - Scope: Gemini adapter process env, auth-required parsing, and adapter docs/tests Duplicate search performed: - `gh pr list --state all --search "gemini headless invocation stall"` found only this PR. - `gh pr list --state all --search "gemini NO_BROWSER NO_COLOR"` found only this PR. - `gh issue list --state all --search "gemini headless authentication"` found related issue #2344 but no exact duplicate. ## What Changed - Set a headless-safe Gemini invocation env at the final child-process boundary: `TERM=xterm-256color`, `COLORTERM=truecolor`, and `NO_BROWSER=1`. - Delete inherited `NO_COLOR` from the child env so Gemini CLI keeps color-capable terminal behavior when deciding whether it can run non-interactively. - Classify Gemini `FatalAuthenticationError: Manual authorization is required...` failures as `gemini_auth_required`. - Update Gemini adapter docs to describe the non-interactive `--prompt` path and headless env behavior. - Add/extend focused parser and remote execution tests for the auth-required and env invariants. ## Verification - `pnpm exec vitest run packages/adapters/gemini-local/src/server/parse.test.ts packages/adapters/gemini-local/src/server/execute.remote.test.ts` — 23 tests passed. - `pnpm --filter @paperclipai/adapter-gemini-local typecheck` — passed. - Local Gemini CLI probe confirmed `NO_BROWSER=1` turns the browser-auth stall into a fast auth error instead of an interactive wait. ## Risks Low risk. The change is limited to `gemini_local` invocation env, parsing, docs, and tests. It does not touch schemas, API routes, persisted data, or other adapters. Operational risk: environments that intentionally rely on `NO_COLOR` being inherited by Gemini child processes will no longer pass that variable through. That is intentional here because Gemini CLI auth/headless behavior should not depend on inherited color suppression. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI GPT-5 Codex via Codex CLI, with shell/file-editing tool use for repository inspection, code edits, tests, and GitHub PR maintenance. ## 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 - [ ] 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 - [ ] 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 /cc @codex — please review. |
||
|
|
277a9a43d6 |
fix(recovery): convert review-parked continuations into dependency waits (#8371)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The productivity-recovery subsystem watches for "stranded" assigned issues — claims whose live run disappeared — and repairs, resumes, or visibly blocks them > - When an executor decomposes an umbrella issue into sub-tasks, it parks its own continuation as "waiting on review/approval" (error code `issue_continuation_waiting_on_review`) — a deliberate pause, not a lost run > - Recovery's staleness gate mistook that deliberate park for a disappeared run: it retried once, then escalated the issue to `blocked` with a recovery action and an operator-facing failure notice — even though nothing had failed and there was nothing for a human to do > - The user is left staring at an inscrutable, over-technical "stranded" error on a task they did nothing wrong with, with no idea what action to take > - This pull request teaches recovery to recognize a review-parked continuation and, when the issue has a real waiting target (open sub-tasks or unresolved blockers), convert it into a first-class dependency wait: `blocked`-by-children, original assignee kept, plus a plain-language comment saying it will resume automatically > - The benefit is that post-decomposition umbrellas sit on a real waiting path and self-resume through the normal blockers-resolved flow, while genuine strands (no waiting target) still escalate exactly as before ## Linked Issues or Issue Description Refs #6503 ## What Changed - `server/src/services/recovery/service.ts`: add `resolveContinuationWaitingOnReview`. When a continuation was cancelled with `issue_continuation_waiting_on_review` and the issue has a real waiting target — open (non-terminal) sub-tasks or existing unresolved blockers — recovery sets the issue `blocked` by those issues, keeps the original assignee, posts a plain-language `system` comment, and logs the activity. Wired into `reconcileStrandedAssignedIssues` ahead of the escalation path, with a new `waitingOnReviewResolved` counter on the result. - With no waiting target, the code falls through to the existing escalation, preserving genuine stranded-run detection. - `server/src/__tests__/heartbeat-process-recovery.test.ts`: two new tests — (1) a review-parked continuation converts into a dependency wait on its open sub-tasks (done children excluded, no recovery issue opened, plain-language comment, raw error code never leaks), and (2) it still escalates when no open dependency remains. - `doc/execution-semantics.md`: document the "Deliberate wait is not a lost run" recovery rule and the requirement that a post-decomposition umbrella hold a first-class waiting path rather than relying on `parentId` rollup. ## Verification - `cd server && npx tsc --noEmit` — passes against current `master`. - New tests in `server/src/__tests__/heartbeat-process-recovery.test.ts` (describe: "heartbeat orphaned process recovery"): - "converts a continuation parked for review into a dependency wait on its open sub-tasks" - "still escalates a continuation parked for review when no open dependency remains" - Run with the repo's vitest setup, e.g. `pnpm vitest run server/src/__tests__/heartbeat-process-recovery.test.ts` (requires the embedded-postgres test harness). ## Risks Low. The change adds a single guarded pre-check ahead of the existing escalation path; behavior is unchanged when the cancellation error code is not `issue_continuation_waiting_on_review` or when the issue has no open sub-task / unresolved blocker to wait on. No schema or migration changes. ## Model Used Claude (Anthropic), Opus-class model, via the Claude Code agent harness — 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 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 - [ ] If this change affects the UI, I have included before/after screenshots (N/A — server-only) - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending CI) - [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 |
||
|
|
8af3bc9ed4 |
fix(ui): prevent mobile viewport horizontal scroll (#8370)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work, with a web UI that must work on mobile as well as desktop. > - The UI layout root in `ui/src/components/Layout.tsx` branches on `isMobile` to choose between a desktop flex column that clips overflow and a mobile container. > - The mobile branch was only `min-h-dvh` — it had no horizontal-overflow guard, while the desktop branch already used `overflow-clip`. > - As a result, any descendant wider than the screen (a long unbreakable token, an over-wide element) pushed the entire viewport sideways, producing horizontal scroll on the whole page. > - This pull request adds `overflow-x-clip` to the mobile root container so stray wide descendants are clipped to the viewport width. > - The benefit is a durable, page-agnostic guard: it prevents this class of bug on every mobile page regardless of which element overflows, without breaking vertical body scroll or the sticky breadcrumb. ## Linked Issues or Issue Description Fixes: #8369 Related (different scope — these fix horizontal scroll *inside* a specific component's own container, not the viewport-level layout guard): Refs #2128. ## What Changed - `ui/src/components/Layout.tsx`: added `overflow-x-clip` to the mobile (`isMobile`) layout root container. - Used `clip` rather than `hidden` deliberately: `clip` leaves `overflow-y` computed as `visible`, so native body scrolling and the sticky breadcrumb keep working; `hidden` would have forced a scroll container and broken them. - `ui/src/components/Layout.test.tsx`: added a regression test asserting the mobile root carries `overflow-x-clip` (and not `overflow-hidden`) and the desktop root carries `overflow-clip`. ## Verification - `pnpm --filter @paperclip/ui test src/components/Layout.test.tsx` → 12 passed (10 existing + 2 new). - Manual: open the web UI at a mobile-width viewport (≤ 768px) on a page with a wide/unbreakable descendant (e.g. a task whose body contains a long unbreakable token). - Before: the entire page scrolls horizontally. - After: the overflow is clipped to the viewport width; the page no longer scrolls sideways, and vertical body scroll plus the sticky breadcrumb continue to work. - The change is a single Tailwind utility on the mobile branch only; desktop layout is unchanged (it already used `overflow-clip`). ## Risks Low risk. The change only adds horizontal-overflow clipping to the mobile layout root. It does not affect the desktop branch, vertical scrolling, or any component internals. Components that need to scroll horizontally manage their own internal overflow and are unaffected. ## Model Used Claude Opus 4 (claude-opus-4) via the Claude Code agent harness, 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 - [ ] 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 - [ ] 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> |
||
|
|
2853a9ae69 |
perf(ci): shard the general-server test lane across 3 runners (#8360)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Every PR runs the `PR` GitHub Actions workflow, whose `verify` gate fans out into parallel test lanes (general tests, serialized server route suites, build, typecheck) > - The `General tests (server)` lane had grown into the run's critical path: it executed all ~213 non-route server suites serially in a single job (~7.2m of test time), more than 2x any other job > - It runs serially because `server/vitest.config.ts` pins `maxWorkers: 1`, so server suites cannot parallelize within a single runner — the only lever is spreading them across runners > - This pull request shards that lane into 3 even partitions that run on separate runners, mirroring the 4-way sharding already used for the serialized route suites > - The benefit is the lane drops from ~7.7m to ~2.4m/shard, cutting overall PR wall time roughly in half (~8.5m → ~4.2m) ## Linked Issues or Issue Description No public GitHub issue exists for this work, so the underlying issue is described inline following the feature-request template. ### Problem or motivation PR CI wall time had crept back up to ~8.5m. On a recent fully-green run, the `General tests (server)` job took 7.72m — more than double any other job and the clear critical path. Of that, 7.23m was pure test execution (dependency install was a cached 0.27m). The job ran all server suites that are not route/authz tests (213 files) one after another, because the server vitest project pins `maxWorkers: 1`, making these suites inherently serial within a single runner. ### Proposed solution Shard the general-server lane across 3 parallel runners — the same technique the route/authz suites already use — so the suite set is split into even, deterministic partitions that run concurrently. Add a regression test that proves the shards always cover the full suite set with no gaps or overlap. ### Alternatives considered - **Raise `maxWorkers` for the server project** to parallelize within one runner — rejected: the server suites share process-level state (DB/port), which is exactly why `maxWorkers: 1` is pinned. - **Two shards instead of three** — would leave the lane at ~3.6m, still above the next bottleneck (Canary Dry Run, ~4.1m wouldn't be the gate). Three lands the lane comfortably below it. - **Do nothing / accept the slow lane** — rejected: it gates every PR. ### Roadmap alignment Developer-experience / CI tooling. Not core product roadmap work; does not overlap with planned features in `ROADMAP.md`. ## What Changed - `scripts/run-vitest-stable.mjs`: the `general-server` general-test group now accepts `--shard-index` / `--shard-count`. It enumerates the full server test set (the whole `server/src` tree, minus the route/authz suites that already run in their own serialized shards) and splits it deterministically by modulo. The non-sharded local invocation (`pnpm test:run:general --group general-server`) is unchanged. - `.github/workflows/pr.yml`: the `general_tests` matrix runs `general-server` as 3 parallel shards (1/3, 2/3, 3/3). Workspace groups are unchanged. The `verify` gate already aggregates the whole matrix result, so the required check name is unaffected. - `scripts/__tests__/run-vitest-stable-shard.test.mjs`: a `node:test` suite asserting the 3 shards form a complete, non-overlapping partition of the general-server set, that no route/authz suite leaks into it, and that shard flags are rejected for the parallel workspace groups. Wired into the `policy` job. ## Verification - New partition test passes locally: `node --test ./scripts/__tests__/run-vitest-stable-shard.test.mjs` (3/3). - Confirmed the 3 shards form a complete, non-overlapping partition of all 213 files (71/71/71). - Ran a live thin shard (3 real server suites, including one outside `__tests__`) — 23 tests passed, confirming positional-include execution works end to end. - This PR's own CI is the authoritative check: all three `General tests (server (n/3))` jobs went green on the prior run, collectively covering every suite the old single job ran. ## Risks - Low risk. No product code changes — only test orchestration and CI matrix. Shard partitioning is deterministic and is now covered by an automated test that fails if the partition ever develops a gap or overlap. Modulo-on-sorted-filenames balances duration reasonably, matching the approach already proven by the serialized route shards. ## Model Used - Claude (Anthropic), `claude-opus-4-8`, extended thinking + tool use (agentic coding via Paperclip). ## 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 - [ ] If this change affects the UI, I have included before/after screenshots (N/A — no UI change) - [x] I have updated relevant documentation to reflect my changes (inline comments explain the sharding rationale) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (pending this PR's run) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending review) - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
4518f272b2 |
fix(release): publish all remaining @paperclipai workspace packages from CI + guard unpublishable edges (#8365)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work, distributed primarily via the `paperclipai` npm package and its `@paperclipai/*` workspace packages. > - The release subsystem (`scripts/release-package-*`) decides which workspace packages CI republishes at the unified calver version each release, and `replaceWorkspaceDeps()` rewrites every internal `workspace:` dependency to that calver version at publish time. > - The gap: a package that publishes from CI can declare a `workspace:` dependency on a package that is NOT enrolled for CI publish. The dependency spec gets rewritten to a calver version that is never actually published, so the dependent becomes uninstallable. > - This shipped for real: a recent change made `@paperclipai/server` depend on `@paperclipai/skills-catalog`, but skills-catalog was not on the calver release train — so canary builds after that merge failed to resolve `@paperclipai/skills-catalog@<calver>` and `npx paperclipai@canary run` broke. > - This PR addresses it durably by (a) putting every remaining internal package on the CI release train so no internal dependency can dangle, and (b) adding a fail-fast guard so this class of break can never ship again. > - The benefit is that canary and stable installs resolve all internal dependencies, and any future unpublishable workspace edge fails the release build with a clear, named error instead of producing a broken package. ## Linked Issues or Issue Description Refs #8327 (introduced the `server -> skills-catalog` runtime dependency that surfaced the gap). No public issue tracks this; describing it in-PR: - **Problem:** After #8327, `npx paperclipai@canary run` failed for builds past the merge because `@paperclipai/skills-catalog` was rewritten to a calver version that was never published (the package was not enrolled for CI publish). Versions before the merge still ran. - **Root cause:** A `publishFromCi:true` package can declare a `workspace:` dependency on a package that is not `publishFromCi:true`; the release-time version rewrite then points at a non-existent published version. ## What Changed - Enrolled every remaining internal package on the calver release train by setting `publishFromCi: true` in `scripts/release-package-manifest.json`: `skills-catalog`, `teams-catalog`, `plugin-workspace-diff`, `plugin-kubernetes`, `plugin-novita-sandbox`. There are now zero `publishFromCi:false` entries. - `skills-catalog`, `teams-catalog`, `plugin-workspace-diff` already existed on npm — CI simply republishes them at calver. - `plugin-kubernetes` and `plugin-novita-sandbox` were not on npm; their one-time first publish was bootstrapped so the `check-release-package-bootstrap` gate passes. - Added `findUnpublishableWorkspaceEdges()` to `scripts/release-package-map.mjs`, wired into `buildReleasePackagePlan()`. The release map build now fails fast (surfaced by the `check` CI already runs) whenever a `publishFromCi:true` package declares a runtime `workspace:` dependency (`dependencies`/`optionalDependencies`/`peerDependencies`) on a non-`publishFromCi:true` `@paperclipai/*` package, naming the offending edge. - Added tests covering positive/negative detection, all three dependency sections, unknown-package edges, off-train edges, and the live manifest. ## Verification - `node --test scripts/release-package-map.test.mjs` → 9/9 pass (includes a test asserting the live manifest has no unpublishable edges). - `node scripts/check-release-package-bootstrap.mjs scripts/release-package-manifest.json` → passes, naming all five newly-enabled packages (all confirmed present on npm). - Confirmed via `npm view` that all five packages resolve on the public registry. ## Risks - Low risk. The change only enrolls already-existing (or freshly-bootstrapped) packages onto the existing release train and adds a build-time validation. No runtime code paths change. The new guard can only *fail* a release that was already going to ship a broken package. ## Model Used Claude Opus 4.7 (claude-opus-4-7), extended thinking, with tool use / 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 - [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 - [x] If this change affects the UI, I have included before/after screenshots (N/A — no UI changes) - [x] I have updated relevant documentation to reflect my changes (N/A — no doc-facing behavior change) - [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 |
||
|
|
6b45b1559d |
refactor: revert headless Chromium provisioning script (#8362)
Deletes scripts/qa/provision-headless-chromium.sh, reverting PR #8349's host-specific Chromium provisioning helper. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
713b3b08f0 |
chore(qa): add no-sudo headless Chromium provisioning script (#8349)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Visual QA and screenshot-based review depend on a working headless browser in agent sandboxes. > - Playwright can provide the Chromium binary, but some sandboxes lack the host shared libraries that Chromium needs to launch. > - Those sandboxes often do not have sudo, so `playwright install-deps` cannot install system packages. > - This pull request adds a user-space provisioning script for the Chromium runtime dependency closure. > - The benefit is that QA can bootstrap a render-capable Chromium environment without requiring root access or changing app runtime code. ## Linked Issues or Issue Description No public issue exists for this exact QA provisioning gap. Problem: in no-sudo Linux sandboxes, Playwright's bundled Chromium may fail to launch because the host is missing required shared libraries such as libatk, libcups, libgbm, libpango, and libasound. Expected behavior: QA agents should be able to provision a local prefix that lets Playwright launch bundled Chromium headlessly and render text without root access. Related public PR: #7301 covers broader runtime tooling/dependency setup, but this PR is a focused userspace script for the no-sudo Chromium dependency case. ## What Changed - Added `scripts/qa/provision-headless-chromium.sh`. - The script downloads the dependency closure for Playwright's Ubuntu 24.04 Chromium runtime packages with `apt-get download`, extracts the `.deb` files into a user-space prefix, and emits an env file that sets `LD_LIBRARY_PATH` and `FONTCONFIG_FILE`. - The script also configures extracted fonts and an isolated font cache so headless rendering paints text. ## Verification - Ran `bash -n scripts/qa/provision-headless-chromium.sh`. - The handoff verification states the script was run from a clean prefix on Ubuntu 24.04 arm64 as a non-root user, then Chromium 145 launched headless and rendered text at 1440x900 and 390x844 in light and dark mode. ## Risks Low risk. This adds a standalone QA utility script and does not modify application runtime, package manifests, CI configuration, migrations, or production code. ## Model Used OpenAI GPT-5 Codex, tool-enabled coding agent with shell and GitHub CLI access. The original script was prepared by an agent workflow; Git Expert amended public-facing references and prepared the PR. ## 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 - [ ] 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] 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> |
||
|
|
fb0e3fb02a |
Fix mention-granted closed issue comments with resume intent (#8350)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Issue comments are part of the control plane boundary between agent collaboration and task mutation. > - Mention grants intentionally let a mentioned peer agent answer on an issue thread without taking ownership of the task. > - Closed issue comments have a second mutation guard because comments can also request resume or reopen behavior. > - A mention-granted comment should stay append-only unless the actor also has mutation authority. > - This pull request preserves the narrow comment path while keeping explicit `resume` and `reopen` intent behind mutation authorization. > - The benefit is a smaller authorization exception with regression coverage for both allowed and denied paths. ## Linked Issues or Issue Description Refs #2884 Related prior authorization work: #8024, #7863, #6113, #7998. Bug fix description: - What happened: a mention-granted non-assignee agent could be treated as authorized for the closed issue comment route without preserving a clean distinction between append-only comments and explicit reopen/resume mutation intent. - Expected behavior: a mention grant allows a plain comment on a closed issue, but `resume: true` or `reopen: true` still requires mutation authorization. - Steps to reproduce: create a closed issue assigned to one agent, give a different agent a valid mention-scoped comment grant, then POST a comment as that agent with and without `resume`/`reopen` intent. - Deployment mode: server route behavior; covered by focused Vitest regression tests. ## What Changed - Preserve the `issue:comment` authorization decision in `POST /api/issues/:id/comments` so the route can identify legitimate mention-grant decisions. - Skip the closed-issue non-assignee mutation fallback only for inert mention-granted comments. - Continue requiring mutation authorization when a mention-granted closed issue comment includes explicit `resume` or `reopen` intent. - Add regression coverage for the allowed inert comment and denied resume/reopen cases. ## Verification - `pnpm vitest run server/src/__tests__/issue-comment-reopen-routes.test.ts` passed locally: 72 tests. ## Risks - Low risk: scoped to the issue comment route and tests for a specific authorization decision reason. - Residual risk: CI should run the full PR suite. ## Model Used OpenAI GPT-5 Codex via Paperclip Git Expert agent, with tool use for code inspection, test execution, Git, and GitHub 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] 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] 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> |
||
|
|
67f97e8fb0 |
fix(server): enforce read auth for single issue comments (#8346)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Issue comments are part of the control-plane audit trail and must respect the same company and issue authorization boundaries as issue reads. > - Mention-scoped commenting gives low-trust agents a narrow way to reply when an authorized assignee mentions them. > - The comment list route already enforces issue-read authorization, but the single-comment read route only checked same-company access before returning a known comment id. > - That created a broken object-level authorization gap for same-company low-trust agents outside the issue boundary. > - This pull request applies the existing issue-read guard to the single-comment route before loading the comment. > - The benefit is consistent comment-read authorization across list and single-comment endpoints, with regression coverage for the low-trust boundary. ## Linked Issues or Issue Description Refs #7389 Bug fix context: - What happened: `GET /api/issues/:id/comments/:commentId` checked company access but did not enforce the issue-read authorization boundary before returning a single comment by known id. - Expected behavior: single-comment reads should use the same issue-read boundary as issue thread list reads. - Steps to reproduce: authenticate as a same-company low-trust agent outside an issue's readable boundary, then request a known comment id via the single-comment endpoint. - Deployment mode: applies to server authorization behavior in authenticated agent API usage. ## What Changed - Added `assertIssueReadAllowed` to the single issue-comment read route before `getComment` is called. - Added mocked route coverage proving peer agents outside the issue-read boundary get `403` and the comment is not loaded. - Added authorization and embedded route coverage for mention-scoped low-trust comment grants so the intended narrow reply path remains allowed. ## Verification - `NODE_ENV=test pnpm exec vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts` — passed, 60 tests. - `NODE_ENV=test pnpm exec vitest run server/src/__tests__/authorization-service.test.ts` — passed, 26 tests. - `NODE_ENV=test pnpm exec vitest run server/src/__tests__/low-trust-red-team-routes.test.ts` — passed, 8 tests. - `git diff --check` — passed. ## Risks Low risk. This reuses the existing issue-read guard for a read endpoint. The main behavioral shift is that same-company actors who cannot read an issue can no longer fetch a known comment id from that issue, which is the intended authorization boundary. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI GPT-5 via Codex, with repository tool use and command execution. Runtime context-window details were not exposed by the 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] 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] 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> |
||
|
|
364f0f5a8d |
fix(server): enforce issue read for issue thread lists (#8331)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents coordinate through issue threads, so issue comments and interactions are part of the task authorization surface. > - Low-trust and boundary-limited agents can receive narrow mention-scoped access, but that must not turn into broad same-company issue-thread reads. > - The comment creation path was narrowed to allow explicit mention replies without granting mutation access. > - The surrounding list/read routes still needed to enforce the same `issue:read` boundary before returning thread data. > - This pull request applies the issue read check to issue comment and interaction listing routes, and locks that behavior with server regressions. > - The benefit is that narrow cross-agent collaboration remains possible without exposing unrelated issue-thread history. ## Linked Issues or Issue Description Bug fix: - What happened: same-company agents outside an issue read boundary could still hit issue-thread listing routes and receive thread data. - Expected behavior: issue comments and issue-thread interactions should only be listed after the actor is allowed to read the issue. - Steps to reproduce: configure a peer agent denied by the issue read boundary, then request `GET /api/issues/:id/comments` or the issue interaction listing route. - Paperclip version/commit: current `master` before this branch. - Deployment mode: applies to server authorization in all modes. Related public context: #7389, #7863, #8024. ## What Changed - Added issue read enforcement before listing issue comments. - Added issue read enforcement before listing issue-thread interactions. - Added server regressions for denied peer-agent issue-thread access while preserving mention-scoped collaboration behavior. - Removed an avoidable per-mentioned-comment issue reload in mention-grant authorization by passing the already-loaded issue assignee through the helper. - Documented cross-agent issue read/comment authorization behavior in the Paperclip API reference. - Added a resilient fallback for pinned external skills when GitHub tree fetches are temporarily unavailable during catalog builds. ## Verification - `pnpm vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts server/src/__tests__/authorization-service.test.ts` - 2 test files passed - 84 tests passed - Existing mocked recovery revalidation warnings were emitted by the route suite and the command exited 0 - Greptile: 5/5 on commit `b73fc323f192dc44f88374c16865008d4348923b`; no unresolved review threads. - `pnpm vitest run packages/skills-catalog/src/catalog-builder.test.ts` - 1 test file passed - 6 tests passed - CI: all visible PR checks are terminal green on commit `b73fc323f192dc44f88374c16865008d4348923b`. ## Risks Low risk. This tightens read authorization on issue-thread listing routes; any caller that depended on same-company access without `issue:read` will now receive 403 and must use an explicit grant or valid issue read path. ## Model Used OpenAI GPT-5 Codex, Codex coding agent environment, tool use and local command execution enabled, reasoning mode active. Context window not exposed by the runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have 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 - [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] 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> |
||
|
|
a71c4b6782 |
[codex] feat(watchdog): add task watchdog control plane (#8339)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The task lifecycle and recovery subsystems decide when agent work is still productive, stalled, or ready for review. > - Existing recovery paths can observe stopped or incomplete work, but there was no first-class per-task watchdog model with scoped review permissions. > - Watchdog follow-ups also need strict boundaries so recovery/status-only runs cannot mutate approvals or perform deliverable work. > - This pull request adds the task watchdog data model, API/service layer, scheduler/review flow, adapter wake context, UI configuration surfaces, and docs. > - The branch has been rebased onto current `paperclipai/paperclip` `master`; the watchdog migration is now ordered after master's latest migrations as `0104_issue_watchdogs`. > - The benefit is a more explicit task-review loop that preserves Paperclip's single-assignee and governance invariants while making stalled work easier to route. ## Linked Issues or Issue Description No linked GitHub issue. Paperclip task: [PAP-11275](/PAP/issues/PAP-11275). ## Problem or motivation Task recovery needs a first-class watchdog path that can inspect stopped work and create scoped follow-ups without bypassing normal task ownership. Board/UI users need a way to configure watchdogs on tasks and see watchdog-related live work. Recovery/status-only runs must remain limited to status reporting and must not create approvals, link approvals, or submit approval comments. ## Proposed solution Add a task-watchdog data model, scheduler/classifier, scoped mutation guard, adapter wake context, API/UI configuration surfaces, and documentation so watchdog agents can review stopped task subtrees under explicit boundaries. ## Alternatives considered Reuse the existing recovery-action flow only. That would keep stopped-work detection implicit, make per-task watchdog assignment harder to expose in the UI, and would not provide a durable scoped-review issue for stalled task trees. ## Roadmap alignment This is Paperclip control-plane lifecycle infrastructure for task execution and recovery. I checked `ROADMAP.md`; this PR does not duplicate an existing planned core item. ## What Changed - Added issue watchdog schema, migration, shared contracts, validators, CRUD API, and service support. - Added task watchdog scheduler/classifier behavior, scoped mutation enforcement, adapter wake context, and default watchdog mandate guidance. - Added UI surfaces for configuring watchdogs on new/existing tasks, viewing watchdog activity, and exposing the experimental setting. - Added docs for the user-facing task watchdog workflow and implementation semantics. - Gated new-task watchdog setup behind `enableTaskWatchdogs` and blocked cheap status-only recovery runs from approval mutations. - Rebased onto current `master` and renumbered the idempotent watchdog migration from the branch-local `0102_issue_watchdogs` slot to `0104_issue_watchdogs`. - Addressed Greptile feedback by loading watchdog classifier input with a recursive subtree query and centralizing the watchdog origin-kind constant. - Added and updated focused server/UI tests for watchdog routes, scheduler/classifier behavior, scope boundaries, live task visibility, settings, and new issue dialog behavior. ## Verification - `pnpm vitest run server/src/__tests__/task-watchdogs-scheduler.test.ts server/src/__tests__/task-watchdogs-classifier.test.ts` - `pnpm vitest run server/src/__tests__/approval-routes-idempotency.test.ts server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts` - `pnpm vitest run ui/src/components/NewIssueDialog.test.tsx` - `pnpm --filter @paperclipai/server typecheck` - `git diff --check` - Verified the PR diff does not include `pnpm-lock.yaml` or `.github/workflows`. ## Risks - Medium risk: this introduces a new task lifecycle surface touching DB schema, server routes/services, adapter wake context, and UI task configuration. - Watchdog scheduling behavior depends on the new experimental setting and runtime context checks behaving consistently across local and production agents. - The watchdog migration is idempotent (`IF NOT EXISTS` / duplicate-object guards) so users who tried the previous branch-local migration number should not get duplicate-object failures. - CI and the second Greptile pass are pending after the latest review-fix push. > 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-class coding agent in the Paperclip workspace. Exact runtime model id and context window were not exposed to the agent; tool use and local command execution were enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots — N/A per Paperclip task instruction: do not add screenshots/images to this PR unless they are specifically part of the work. - [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: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
8f4b491d9a |
fix(ui): fix blank page when creating an agent (#8336)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Creating an agent starts at **New Agent → "manually" → pick an
adapter**, which routes to
`/{company}/agents/new?adapterType=claude_local` and renders the
`NewAgent` page with the `AgentConfigForm`
> - `AgentConfigForm` hands the parent a `triggerTestEnvironment`
callback via an `onTestActionChange` effect so the page can wire up its
"Test"/"Save + Test" button
> - That trigger was rebuilt on every render: it depended on
`runEnvironmentTest`, which is derived from a react-query `useMutation`
result, and `useMutation` returns a **brand-new result object identity
on every render**
> - So the `onTestActionChange` effect re-fired every render and pushed
a new function into the parent's state, producing an infinite `setState`
loop ("Maximum update depth exceeded") that threw during render
> - The app's custom router updates location **without remounting**, and
there was no error boundary around the routed outlet, so the throw left
a dead render tree — a fully **blank page** that stayed blank on
back-navigation until a hard refresh
> - This pull request stabilizes the trigger with a latest-ref pattern
so the effect no longer re-fires, and adds a route-keyed error boundary
so any future render throw degrades to a recoverable error card instead
of a blank screen
> - The benefit is that creating an agent works again, and render-time
failures anywhere in the routed UI are contained and recoverable rather
than silently blanking the app
## Linked Issues or Issue Description
No public GitHub issue exists, so the underlying bug is described inline
following the bug report template
(`.github/ISSUE_TEMPLATE/bug_report.yml`):
### What happened?
In the UI, choosing **New Agent → "manually" → (any adapter, e.g.
Claude)** navigates to the agent-config page and renders a **completely
blank page**. The browser console shows React's `Maximum update depth
exceeded`. Using the back button changes the URL but the page stays
blank until a full hard refresh.
### Expected behavior
Selecting an adapter shows the agent configuration form so the agent can
be created.
### Steps to reproduce
1. Open the app and click **New Agent**.
2. Choose **manually**.
3. Pick an adapter (e.g. Claude / `claude_local`).
4. Observe the blank page (URL becomes
`/{company}/agents/new?adapterType=claude_local`).
### Paperclip version or commit
Reproduces on `master` (base of this PR).
### Deployment mode
Reproduces regardless of deployment mode — it is a client-side render
loop.
### Root cause
`useMutation` returns a new result object identity each render, so the
`runEnvironmentTest`-derived `triggerTestEnvironment` callback was
unstable, which made the `onTestActionChange` effect push a new function
into parent state every render → infinite update loop → render throw →
no boundary → blank tree.
## What Changed
- **`ui/src/components/AgentConfigForm.tsx`** — Stabilize the
environment-test trigger handed to the parent using a latest-ref
pattern: the churny behavior (`runEnvironmentTest`,
`testEnvironmentDisabled`) lives in a `useRef` updated by an effect, and
the exposed `triggerTestEnvironment` is a `useCallback(() =>
triggerRef.current(), [])` with an empty dep array, so its identity is
stable across renders and the `onTestActionChange` effect no longer
re-fires every render.
- **`ui/src/components/RouteErrorBoundary.tsx`** (new) — A route-keyed
React error boundary that catches render throws and renders a
recoverable error card (showing the error message, with "Go back" and
"Reload page" actions). It resets automatically when the route
(`pathname + search`) changes.
- **`ui/src/components/Layout.tsx`** — Wrap the routed `<Outlet />` in
`<RouteErrorBoundary>` so a render throw degrades to the error card
instead of a blank page.
- **`ui/src/components/RouteErrorBoundary.test.tsx`** (new) — Regression
test: a throwing child is contained as a recoverable error card (showing
the message), "Go back" calls `navigate(-1)`, and the boundary resets to
render children again after the route changes.
## Verification
- `npx vitest run ui/src/components/RouteErrorBoundary.test.tsx` → 3
passed (regression test for this fix).
- `npx vitest run ui/src/components/AgentConfigForm.test.ts` → 9 passed.
- `npx tsc -b` in `ui` → 0 errors.
- Manual: ran the dev server, clicked **New Agent → manually → Claude**.
- **Before:** blank page; console logs `Maximum update depth exceeded`;
back button leaves the page blank until hard refresh.
- **After:** the agent configuration form renders normally and the agent
can be created; navigating away and back works without a hard refresh.
- Boundary check: with the loop still in place (pre-fix), the new
boundary catches the throw and shows a recoverable error card instead of
a blank screen; "Go back" / route change resets it.
_Screenshots: the before-state is the React `Maximum update depth
exceeded` error and a blank `/agents/new` page; the after-state is the
rendered agent-config form. Both were observed locally; rendered images
can be attached on request._
## Risks
- **Low risk.** Changes are confined to three UI files with no API,
schema, or behavioral change to agent creation beyond fixing the loop.
- The latest-ref pattern preserves identical runtime behavior of the
test trigger (same guard, same `runEnvironmentTest()` call) — it only
stabilizes the callback identity.
- The error boundary is additive; on the happy path it renders its
children unchanged. Its only behavior is to catch render throws that
previously blanked the app.
## Model Used
Claude (Anthropic), model `claude-opus-4-8` — extended-thinking-capable,
tool-use (file edit, shell, tests). Used to diagnose the infinite render
loop, implement the latest-ref fix and route error boundary, and verify
via local typecheck/tests.
## 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 (described textually in Verification — see note)
- [ ] I have updated relevant documentation to reflect my changes (N/A —
bug fix, no docs affected)
- [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
|
||
|
|
6a3f5b685d |
fix(agents): clear inbox hire approval when approving/terminating from detail page (#8340)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Hiring a new agent creates a `hire_agent` approval record; until it is resolved, the agent sits in `pending_approval` and the open approval surfaces an "Approve/Reject" card in the inbox > - There are two places to act on that hire: the inbox approval card and the agent detail page's Approve/Terminate buttons > - The agent detail page only flipped the agent to `idle` via `activatePendingApproval`, never resolving the linked approval record > - So after approving (or terminating) from the detail page, the approval stayed `pending` and the inbox kept showing a stale "Approve/Reject" card for an agent that was already decided > - This pull request routes the detail-page approve through the shared `approvalsSvc.approve()` (which resolves the approval and runs activation, budget policy, and the hire-approved notification), and rejects the linked approval when a still-pending agent is terminated > - The benefit is a single source of truth: deciding a hire in one place clears it everywhere, so the inbox no longer asks you to approve an agent you already approved ## Linked Issues or Issue Description No public GitHub issue exists for this; describing in-PR (bug report). **What happened:** Create a new agent, then approve it from the agent detail page. The agent activates, but the inbox still shows a "Hire Agent" item with Approve/Reject buttons for it. **Expected:** Once a hire is approved or rejected anywhere in the UI, it should be resolved everywhere — the inbox should not re-ask you to approve an agent that is already decided. **Root cause:** `POST /agents/:id/approve` called `activatePendingApproval`, which only changes the agent's status. The linked `hire_agent` approval row stayed `pending`. The inbox lists approvals filtered to unresolved statuses, so the card persisted. Terminating a pending agent from the detail page had the mirror problem for the "reject" half. Related (not duplicates): #215 (join-request inbox badge), #1815 (approval detail button loading text). ## What Changed - Added `approvalService.findOpenHireApprovalForAgent(companyId, agentId)` to locate the open `hire_agent` approval for an agent. The company/type/open-status **and** `payload->>'agentId'` predicates all run in SQL (jsonb operator), so the DB returns only the relevant row instead of filtering in JS (`server/src/services/approvals.ts`). - `POST /agents/:id/approve` resolves the linked approval through the shared `approvalsSvc.approve()` — running activation, budget-policy upsert, and the hire-approved notification as one path — and falls back to direct `activatePendingApproval` only when no open approval exists (legacy agents created before approvals were tracked) (`server/src/routes/agents.ts`). - `POST /agents/:id/terminate` now branches the same way: when a still-`pending_approval` agent has an open hire approval, it delegates to `approvalsSvc.reject()` (which resolves the approval **and** terminates the agent internally) and re-reads the agent, otherwise it terminates directly. This avoids terminating the agent twice (`reject()` already calls `agentsSvc.terminate()`) (`server/src/routes/agents.ts`). - The `agent.approved` activity log records the resolved `approvalId` (or `null` on the fallback path) for traceability. ## Verification - `pnpm --filter @paperclipai/server typecheck` — clean. - Targeted server tests pass (57 tests): `npx vitest run src/__tests__/approvals-service.test.ts src/__tests__/agent-permissions-routes.test.ts` - `findOpenHireApprovalForAgent` returns the row the SQL filter yields / returns null when none matches. - Approving from the detail page resolves the linked approval via `approvalsSvc.approve()` and does not double-activate; legacy fallback still calls `activatePendingApproval`. - Terminating a still-pending agent with an open approval calls `approvalsSvc.reject()` and does **not** call `agentsSvc.terminate()` a second time; terminating with no open approval terminates directly. - Manual: create an agent → inbox shows the hire card → approve from the agent detail page → inbox card is gone and the agent is active. Same for terminate-while-pending clearing the card. ## Risks Low risk, server-only, no schema or migration changes. The fallback to `activatePendingApproval` preserves existing behavior for agents with no tracked approval record, so legacy agents still activate. The shared approval path is the same one the inbox card already uses, so approve/terminate-from-detail-page now match approve/reject-from-inbox exactly. ## Model Used Claude Opus 4.8 (claude-opus-4-8), extended thinking with tool use, 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] 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 (server-only change, no UI diff) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (in progress) - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
7069053a1f |
[codex] Add ask issue work mode (#8334)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Issue work mode controls how a task starts and how the conversation composer frames the operator's intent. > - Paperclip already supports standard agent execution and planning mode, but there is no lightweight mode for asking a question without immediately implying execution or plan drafting. > - That gap makes low-commitment clarification workflows look like normal task execution. > - This pull request adds an explicit Ask mode and threads it through shared contracts, server heartbeat context, and the issue composer UI. > - The benefit is that operators can create or switch a task into a question-oriented mode while preserving existing agent and planning flows. ## Linked Issues or Issue Description No public GitHub issue exists for this change. Inline feature request follows the repository feature request template. ### Subsystem affected Cross-cutting: `packages/shared`, `server/`, and `ui/`. ### Problem or motivation Issue conversations currently distinguish standard agent work from planning work, but question-first conversations do not have a clear public mode in the shared contract or UI. Operators who want to ask an agent a focused question have to use standard mode, which can imply normal task execution, or planning mode, which asks for a plan rather than an answer. ### Proposed solution Add Ask as a first-class issue work mode. It should be selectable from issue creation and issue chat, cycle alongside Standard and Planning from the keyboard shortcut/menu, appear distinctly in composer styling, and be included in heartbeat context so agents know to answer directly instead of executing or drafting a plan. ### Alternatives considered - Keep using standard mode for questions: rejected because it does not communicate answer-only intent to the agent or the UI. - Reuse planning mode for questions: rejected because planning mode asks for a plan and is semantically different from asking a question. - Add only local UI copy: rejected because the mode needs to be represented in the shared contract and server heartbeat context to be reliable. ### Roadmap alignment This is a focused issue-workflow improvement. `ROADMAP.md` was checked and no duplicate planned core work was found. ### Additional context Related public searches performed before opening this PR: - GitHub PR search for `"ask mode" repo:paperclipai/paperclip` - GitHub issue search for `"ask mode" repo:paperclipai/paperclip` - GitHub PR search for `"work mode" "ask" repo:paperclipai/paperclip` No duplicate PR was found. ## What Changed - Added `ask` to the shared issue work-mode contract and validation coverage. - Included issue work mode in heartbeat context summaries so agents can see standard, planning, and ask state. - Added Ask mode metadata, styling, composer tone handling, and selection/cycling behavior in the issue chat/new issue UI. - Updated focused tests for shared validators, heartbeat context, and affected UI work-mode flows. ## Verification - `NODE_ENV=test pnpm exec vitest run ui/src/components/ChatComposer.test.tsx ui/src/components/IssueChatThread.test.tsx ui/src/components/NewIssueDialog.test.tsx ui/src/lib/work-mode-meta.test.ts` - `NODE_ENV=test pnpm exec vitest run packages/shared/src/validators/issue.test.ts server/src/__tests__/heartbeat-context-summary.test.ts server/src/__tests__/issues-service.test.ts ui/src/components/ChatComposer.test.tsx ui/src/components/IssueChatThread.test.tsx ui/src/components/NewIssueDialog.test.tsx ui/src/lib/work-mode-meta.test.ts ui/src/pages/IssueDetail.test.tsx` The broader targeted command passed 8 test files / 245 tests. Visual reference for Standard/Planning/Ask composer states: https://gist.github.com/cryppadotta/714d8590bac55500a65e7e16de5bb4b8 It emitted an expected warning from an existing server test fixture about a missing run-log fixture while verifying derived issue comment metadata. ## Risks Low to moderate risk. This adds a new enum value that crosses shared, server, and UI contracts. Existing standard and planning modes are preserved, but any downstream code assuming only two non-terminal work modes may need to handle `ask`. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI GPT-5 Codex coding agent in Paperclip CodexCoder mode, with shell, git, GitHub connector, and local test execution tools. Context window and exact hosted model snapshot are not exposed in this runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have 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/...`, `feat/...`) 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] 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] 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> |
||
|
|
aeea5f9195 |
fix(ui): stabilize routine schedule editor and interrupted run labels (#8333)
## Thinking Path > - Paperclip is the open source control plane people use to manage AI agents for work. > - This change touches the board UI surfaces for issue run timelines and routine schedule editing. > - Operators need cancelled runs to distinguish ordinary cancellation from human interruption, otherwise the run history reads as more severe than it is. > - Routine schedule editing also needs to preserve user-entered cron values while rendering common schedules in a stable, understandable editor. > - This pull request keeps the editor state tied to explicit schedule values, adds coverage for routine editable sections, and makes interrupted run copy more precise. > - The benefit is less surprising routine editing and clearer issue run history for operators. ## Linked Issues or Issue Description No public GitHub issue is filed for this exact branch. Related public PRs: - Refs #3581, which addresses a narrower schedule reset case. - Refs #1803, which is another open schedule editor UI improvement. Problem description: Routine trigger schedules can be edited through the board UI, but the previous schedule editor path could normalize or reset cron state in ways that made unsaved edits fragile. Issue run history also labeled operator-interrupted cancelled runs like ordinary cancellations. Reviewers should treat this PR as a combined UI stabilization pass for those two visible operator workflows. ## What Changed - Added a more stable routine schedule editor flow that preserves explicit cron values and handles custom/common schedule transitions. - Wired routine editable-section state so schedule drafts do not get overwritten by unrelated section refreshes. - Added tests for schedule editor behavior, routine editable sections, and routine service schedule preservation. - Updated issue run timeline copy so operator-interrupted cancelled runs display as interrupted, while ordinary cancelled runs remain cancelled. - Kept the classic issue thread run label behavior aligned with the current issue thread surface. ## Verification - `NODE_ENV=development pnpm run preflight:workspace-links && NODE_ENV=development pnpm exec vitest run server/src/__tests__/routines-service.test.ts ui/src/components/IssueChatThread.test.tsx ui/src/components/ScheduleEditor.test.tsx ui/src/components/routine-sections/editable-sections.test.tsx` — 103 tests passed. Note: direct `pnpm exec vitest ...` without `NODE_ENV=development` loaded a React build where `React.act` is undefined in this workspace. The same targeted tests pass under the development React build. ## Risks Low to medium risk. The changes are UI-focused but touch routine schedule editing, which is a high-frequency operator workflow. The main risk is that an uncommon cron expression could render as custom when a user expected a preset; the added tests cover preservation and explicit custom handling. > 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-class coding agent. Exact hosted runtime model ID and context window were not exposed in this session. Tool use and local command execution were used for inspection, verification, GitHub PR creation, and Paperclip issue updates. ## 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 - [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: Paperclip <noreply@paperclip.ing> |
||
|
|
76ffa5023f |
refactor(ui): rename environment probe button from "Test draft" to "Test" (#8337)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run inside environments, and the company environments UI lets a user configure and validate a new environment before saving it > - For non-local drivers, that form shows a button to probe/test the environment configuration > - The button was labeled "Test draft", which is confusing — the user is just testing the environment they're configuring, and "draft" adds no meaning > - This pull request renames the button label from "Test draft" to "Test" > - The benefit is a clearer, less cluttered action that matches what the button actually does ## Linked Issues or Issue Description No public GitHub issue exists. Inline bug description (per CONTRIBUTING.md → "Link Issues or Describe Them In-PR"): ### What happened? In company environment creation/editing, the environment-probe button for non-local drivers reads "Test draft", which is confusing. ### Expected behavior The button should simply read "Test". ### Steps to reproduce 1. Open the company environments page. 2. Add or edit an environment with a non-local driver. 3. Observe the probe action button reads "Test draft" instead of "Test". ### Paperclip version or commit `master` — probe button in `ui/src/pages/CompanyEnvironments.tsx`. ### Deployment mode Local dev (pnpm dev). ## What Changed - Renamed the environment-probe button label from "Test draft" to "Test" in `ui/src/pages/CompanyEnvironments.tsx`. - The in-flight/pending state is unchanged and still reads "Testing...". ## Verification - Open the company environments page, add or edit an environment with a non-local driver, and confirm the action button reads **Test** (and **Testing...** while a probe is in flight). - This is a single string-literal change in JSX with no logic change; CI typecheck/build/test gates cover regressions. Before → After (button label): `Test draft` → `Test` ## Risks Low risk — UI label-only change, no behavior or logic affected. ## Model Used - Provider: Anthropic (Claude) - Model: `claude-opus-4-8` (Claude Opus) - Capabilities: extended thinking, tool use / 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 - [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] No test is required: this is a label-only change titled `refactor(ui):`, which the repo's `check-pr-test-coverage` policy exempts from the test requirement - [x] If this change affects the UI, I have documented the label change (before/after text above). A rendered screenshot is a non-blocking Greptile P2 recommendation; Greptile rated the PR 5/5 "safe to merge" - [x] I have updated relevant documentation to reflect my changes (none required) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (re-running commitperclip after retitle + description fix) - [x] Greptile is 5/5 (one non-blocking P2 screenshot recommendation noted above) - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
a2abebee07 |
docs: require descriptive, instance-free PR branch names (#8312)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributions flow through GitHub PRs against `paperclipai/paperclip`, but most contributors author their work inside their own private Paperclip instance > - That tooling names a working branch after the internal issue/task — e.g. `PAPA-42-why-did-this-break` > - When that branch is pushed for a PR, the public PR's head branch carries instance-local context: it's not meaningful to reviewers and leaks instance-derived identifiers > - We just documented the issue-reference side of this (#8292), but said nothing about branch names > - This pull request adds a **Branch Naming** section to `CONTRIBUTING.md` and a matching PR-template checklist item, telling contributors to rename branches to descriptive, change-scoped names before pushing > - The benefit is public PRs whose branch names describe the change and contain no instance-local details ## Linked Issues or Issue Description Fixes: #8311 Refs: #8292 ## What Changed - `CONTRIBUTING.md`: new **Branch Naming** subsection — rename instance-named branches (e.g. `PAPA-42-...`) to descriptive, change-scoped, kebab-case names before pushing, with examples and the rename commands. - `.github/PULL_REQUEST_TEMPLATE.md`: new checklist item asserting the branch name describes the change and contains no internal ticket id or instance-derived details. ## Verification Docs-only change. Rendered Markdown locally and confirmed the new **Branch Naming** section and the new checklist item appear as intended. This PR's own branch (`docs/contributor-branch-naming`) follows the new guidance. No code paths affected. ## Risks Low risk — documentation and PR-template text only; no runtime, build, or schema impact. Automatically renaming branches at push time in the tooling is intentionally out of scope and tracked as separate follow-up work. ## Model Used Claude Opus 4.7 (claude-opus-4-7), extended thinking, agentic tool use 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 and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [ ] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [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> |
||
|
|
f13fd1157b |
feat(adapters): stamp agent id via X-Anthropic-Agent-Id for claude_local (#8322)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Local agents run through the `claude_local` adapter, which spawns Claude Code and routes its Anthropic API traffic through a shared proxy (e.g. better-ccflare). > - That proxy's request log feeds observability dashboards that break down cost and token usage. > - But requests from local agents arrive without an agent identifier, so all traffic collapses into a single unattributed bucket and per-agent telemetry is impossible. > - Anthropic's CLI forwards `ANTHROPIC_CUSTOM_HEADERS` onto each API request, and better-ccflare now reads an `X-Anthropic-Agent-Id` header to attribute requests per agent. > - This pull request stamps the agent's id into `ANTHROPIC_CUSTOM_HEADERS` as `X-Anthropic-Agent-Id`, injected into the env the adapter actually forwards to the spawned process. > - The benefit is per-agent cost/token attribution in proxy-backed dashboards, with zero impact on agents that don't run behind such a proxy. ## Linked Issues or Issue Description No public GitHub issue exists; describing inline per CONTRIBUTING.md (feature template). **Problem or motivation** When several `claude_local` agents share a proxy (better-ccflare) in front of the Anthropic API, the proxy cannot tell which agent issued a given request. Per-agent cost and token dashboards therefore can't be built — every request is attributed to one undifferentiated bucket. **Proposed solution** Have the `claude_local` adapter stamp each spawned Claude Code process with `ANTHROPIC_CUSTOM_HEADERS: X-Anthropic-Agent-Id: <agentId>`. The Anthropic CLI forwards that header on every API call, so the proxy can attribute requests to the specific agent. **Alternatives considered** Parsing per-agent identity from workspace paths or session ids in the proxy — brittle and proxy-specific. A first-class request header is the stable contract; it pairs with better-ccflare's `X-Anthropic-Agent-Id` support ([tombii/better-ccflare#260](https://github.com/tombii/better-ccflare/pull/260)) and dashboards like [danieltamas/axon](https://github.com/danieltamas/axon). **Roadmap alignment** Additive observability glue for an existing adapter; no overlap with planned core work. ## What Changed - Added `server/src/adapters/claude-agent-id-header.ts` exporting `stampClaudeAgentIdHeader`, which wraps the `claude_local` execute and merges `X-Anthropic-Agent-Id: <agentId>` into the run's `config.env.ANTHROPIC_CUSTOM_HEADERS`. - `server/src/adapters/registry.ts`: the `claude_local` adapter now uses the wrapped execute (`stampClaudeAgentIdHeader(claudeExecute)`). - The header is merged into **`config.env`** — the env the Claude adapter actually forwards into the spawned process — rather than `agent.adapterConfig.env`, which is resolved upstream before `execute` runs and is never read by the Claude adapter. - `agentId` is sanitized (CR/LF stripped, capped at 256 chars) before interpolation to prevent HTTP header injection. - Passthrough when no agent id is available; a pre-existing `X-Anthropic-Agent-Id` (manual override) is respected and not duplicated. - Added `server/src/adapters/claude-agent-id-header.test.ts` covering injection into `config.env`, append-to-existing `ANTHROPIC_CUSTOM_HEADERS`, CR/LF sanitization, length bounding, no-agent-id passthrough, and the duplicate-header guard. ## Verification - Unit tests: `cd server && npx vitest run src/adapters/claude-agent-id-header.test.ts` → 6 passing. - CI: typecheck, build, server suites, and Greptile gates run on this PR. - Manual: configure a `claude_local` agent behind better-ccflare, run a heartbeat, and confirm the proxy log shows `X-Anthropic-Agent-Id: <agentId>` on that agent's requests. An agent that already sets `X-Anthropic-Agent-Id` in its adapter config keeps its override (no duplicate line). ## Risks Low risk. The change is additive and non-breaking: - When no agent id is present, or when an `X-Anthropic-Agent-Id` is already configured, execution is byte-for-byte unchanged (the original `ctx` is passed straight through). - The header only has an effect for deployments whose proxy reads it; agents without such a proxy are unaffected. - The agent id is sanitized (CR/LF removed, length-bounded) before it is interpolated into a header value, preventing header injection. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. This is additive observability glue for an existing adapter and does not overlap planned core work. ## Model Used Claude (Anthropic) — Claude Opus 4.8 (`claude-opus-4-8`), via the Claude Code CLI with extended reasoning 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] 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 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: zenprocess <zenprocess@users.noreply.github.com> Co-authored-by: Devin Foley <devin@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
a0c7e38ccd |
fix(ui): reorder environment driver dropdown and drop Local option (#8329)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run on **environments**, configured in Company Settings → Environments, where each environment has a **driver** (how/where it runs) > - The "new environment" form exposed a Driver `<select>` ordered `SSH → Sandbox → Local`, with **Local** as a selectable create option > - You can only ever have one local environment (it's the host running Paperclip), so offering **Local** in the create flow is misleading — and Sandbox is the most common choice, yet it sat in the middle of the list > - This pull request removes the **Local** option from the create form and moves **Sandbox** to the top of the list (`Sandbox → SSH`) > - The benefit is a create flow that only offers drivers you can actually create, with the most-used driver first ## Linked Issues or Issue Description No public GitHub issue exists. Describing the underlying problem inline, following the **Feature request** issue template: ### Problem or motivation When creating a new environment (Company Settings → Environments → New environment), the Driver `<select>` lists three options in this order: `SSH`, `Sandbox`, `Local`. **Local** is selectable even though a local environment represents the Paperclip host itself and cannot be created more than once, so choosing it in the create flow is not a valid action. **Sandbox**, the most common choice, sits in the middle of the list instead of first. ### Proposed solution Drop **Local** from the create dropdown and order the remaining options **Sandbox** first, then **SSH**. The create flow then only offers drivers you can actually create, with the most-used driver surfaced first. Existing local environments must still render and be editable, so the `"local"` driver value is retained in the type union. ### Alternatives considered Keeping **Local** but disabling it: rejected — a permanently-disabled option is noise and still implies local environments are creatable here. Hiding the whole driver field when only one option remains: rejected — both Sandbox and SSH remain valid, so the selector is still needed. ### Roadmap alignment Not roadmap-tracked. This is a small, self-contained UX correction to an existing form, not new core feature work. ## What Changed - Removed the **Local** `<option>` from the new-environment Driver `<select>`. - Reordered the remaining options to **Sandbox** (when sandbox creation is enabled) then **SSH** (was `SSH → Sandbox → Local`). - Simplified the now-dead `local` branch in the select's `onChange` handler (`driver` resolves to `sandbox` or `ssh` only). - Updated the Driver field hint text to describe only Sandbox and SSH. - Kept the `"local"` value in the `driver` type union so existing local environments still render/read correctly — only the create-form option was dropped. - Added a unit test asserting the driver options omit `local` and list `sandbox` before `ssh`. ## Verification - `pnpm --filter ./ui typecheck` (`tsc -b`) — passes. - `pnpm --filter ./ui vitest run src/pages/CompanySettings.test.tsx` — 3 passed (includes the new assertion). Driver option ordering (create form): | | Before | After | |---|---|---| | 1 | SSH | Sandbox* | | 2 | Sandbox* | SSH | | 3 | Local | — (removed) | *Sandbox appears when at least one run-capable sandbox provider plugin is installed. Note on screenshots: the Driver control is a native `<select>`; its expanded option list is OS-rendered and cannot be captured in a page screenshot. The before/after option order is shown above and locked in by the new unit test. ## Risks Low risk. Pure create-form UI change. The `"local"` driver type is retained for reading/editing existing environments, so no existing environment is affected. No API, schema, or migration changes. ## Model Used Claude Opus 4.8 (`claude-opus-4-8`), extended reasoning with tool use, 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 - [ ] If this change affects the UI, I have included before/after screenshots — native `<select>` reorder; expanded list isn't screenshot-capturable. Before/after option order documented above and covered by a unit test. - [ ] I have updated relevant documentation to reflect my changes — no user-facing docs cover this dropdown - [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> |
||
|
|
af22abc80e |
chore(lockfile): refresh pnpm-lock.yaml (#8332)
Auto-generated lockfile refresh after dependencies changed on master. This PR only updates pnpm-lock.yaml. Co-authored-by: lockfile-bot <lockfile-bot@users.noreply.github.com> |
||
|
|
5e086cb828 |
fix(server): resolve published skills catalog package root and fallback (#8327)
## Thinking Path > - Paperclip is the open source control plane people use to run and supervise AI-agent companies. > - The skills system is part of that core operator experience because agents and humans both depend on the catalog-backed Skills Manager surfaces. > - In source checkouts, the server can find the catalog manifest and bundled skill files through monorepo-relative paths, but published installs do not preserve that layout. > - That mismatch makes `GET /api/skills/catalog` fail in npm/pnpm installs even though the catalog package itself is present. > - The server therefore needs to resolve the catalog from the published `@paperclipai/skills-catalog` package first, while still keeping a monorepo fallback for local development. > - This pull request makes the published package the primary resolution path, uses the same resolved package root for bundled skill file reads, and degrades the list route safely when the manifest is unavailable. > - The benefit is that Skills Manager catalog reads behave correctly in packaged installs instead of only in repo-local development layouts. ## Linked Issues or Issue Description Fixes #8316 Refs #7281 Refs #7313 Refs #7350 Refs #7860 Refs #8223 Refs #8227 ## What Changed - Added `@paperclipai/skills-catalog` as a runtime dependency of `@paperclipai/server`. - Exported `./package.json` from `@paperclipai/skills-catalog` so the server can resolve the published package root directly. - Updated `server/src/services/skills-catalog.ts` to resolve the manifest and package root from the published package first, with the monorepo path retained only as a development fallback. - Applied that resolved package root to bundled catalog file reads so manifest lookup and skill-file reads use the same published layout. - Added `listCatalogSkillsOrEmpty()` so `GET /api/skills/catalog` returns `[]` and logs a warning when the manifest is unavailable instead of surfacing a 500. - Added targeted server tests for published-package resolution and missing-manifest fallback handling. ## Verification - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/skills-catalog-service.test.ts src/__tests__/company-skills-routes.test.ts` - `pnpm --filter @paperclipai/server build` - Packaging smoke: - pack `@paperclipai/shared` and `@paperclipai/skills-catalog` from this checkout - mount those packed artifacts under `server/dist/node_modules` - import `server/dist/services/skills-catalog.js` - verify a bundled catalog `SKILL.md` resolves and reads successfully from the packed package layout ## Risks - Low risk: the change is narrowly scoped to catalog package resolution and fallback behavior. - The new `./package.json` export slightly broadens the catalog package's public surface, so reviewers should confirm that is an acceptable runtime contract. - The empty-array fallback intentionally changes failure mode for a missing manifest from `500` to a warning + empty payload, which is safer for packaged installs but could hide packaging regressions if logs are not monitored. ## Model Used - OpenAI Codex via Paperclip `codex_local` (GPT-5-based coding agent; exact backend model ID/context window not exposed in this harness), with tool use, shell execution, git, and local test/build 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] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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 |
||
|
|
0186e66c7c |
test(SAG-4328): de-flake checkout-lock race in orphan-recovery test (#8315)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server heartbeat subsystem tracks agent runs and recovers orphaned processes via `reapOrphanedRuns` > - When a local process dies mid-run, the recovery path queues a retry run and releases the checkout lock asynchronously > - The test `queues exactly one retry when the recorded local pid is dead` verifies both the retry state and that the checkout lock is released > - The `checkoutRunId` null-check was a bare synchronous assertion, but the checkout-lock release is async — so the test could read `checkoutRunId` before the write committed > - This PR wraps the predicate already gating on `executionRunId` to also wait for `checkoutRunId === null`, so both writes are committed before the assertions fire > - The benefit is a deterministic test that does not race the async checkout-lock release ## Linked Issues or Issue Description - ## What Changed - `server/src/__tests__/heartbeat-process-recovery.test.ts`: Extended the `waitForValue` predicate (line ~1024) from `executionRunId === retryRun?.id` to also require `checkoutRunId === null` before returning the row. This eliminates the race between the assertion and the async terminal-run checkout-lock release. ## Verification - No production code changed — test file only. - Ran the test file 10× locally with `pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts --no-file-parallelism --maxWorkers=1`: ``` Run 1: Test Files 1 passed (1) | Tests 57 passed (57) Run 2: Test Files 1 passed (1) | Tests 57 passed (57) Run 3: Test Files 1 passed (1) | Tests 57 passed (57) Run 4: Test Files 1 passed (1) | Tests 57 passed (57) Run 5: Test Files 1 passed (1) | Tests 57 passed (57) Run 6: Test Files 1 passed (1) | Tests 57 passed (57) Run 7: Test Files 1 passed (1) | Tests 57 passed (57) Run 8: Test Files 1 passed (1) | Tests 57 passed (57) Run 9: Test Files 1 passed (1) | Tests 57 passed (57) Run 10: Test Files 1 passed (1) | Tests 57 passed (57) ``` 10/10 green streak — race eliminated. ## Risks Low risk — test-only change. No production code modified. The predicate change only narrows the waitForValue poll to require both writes committed before asserting, matching the intent already documented in the comment on line 1031. > 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 Claude Sonnet 4.6 (claude-sonnet-4-6) — Anthropic, 200K context, 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] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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: Director of Engineering <cto@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Devin Foley <devin@paperclip.ing> |
||
|
|
9cd62cf3bb |
fix(cli): handle headless browser-open failure in board auth (#8328)
## Thinking Path > - Paperclip is the open-source app for managing AI agents, very commonly self-hosted as a headless Docker container. > - Its CLI authorizes board/admin actions through an interactive browser-approval flow (`auth login`, and the auto-recover path behind commands like `run list`). > - That flow calls `openUrl()`, which `spawn`s the OS browser opener (`xdg-open` on Linux). > - In a headless container there is no `xdg-open`; `spawn` reports the missing binary asynchronously via an `'error'` event, which the surrounding `try/catch` cannot catch, so Node aborts the process before the approval can be polled. > - This makes CLI board auth impossible in the most common self-hosted deployment shape. > - This PR makes `openUrl` error-handled, async, and truthful, and adds headless affordances. > - The benefit is that board-authenticated CLI commands degrade gracefully and work headless instead of crashing. ## Linked Issues or Issue Description Closes #7941 ## What Changed - `openUrl` (`cli/src/client/board-auth.ts`) is now async and attaches an `'error'` listener to the spawned opener: resolves `false` on async spawn failure (missing binary) or sync throw, and `true` only on a successful `'spawn'`. Fixes the unhandled-`'error'` crash and makes the return value honest. - `loginBoardCli` prints an accurate "couldn't open a browser" message, supports `--no-browser` / `PAPERCLIP_NO_BROWSER` to skip the open attempt, and renders the approval URL from `PAPERCLIP_PUBLIC_URL` (or `publicBaseUrl`) so it's reachable from a remote operator's browser. - Updated the three other `openUrl` call sites (`cloud.ts` ×2, `company.ts`) to `await` it. - Added the `auth login --no-browser` flag. - Tests: new `open-url.test.ts` (launch → true, async ENOENT → false, sync throw → false); extended auth-command-registration test for `--no-browser`. ## Verification - `pnpm --filter paperclipai typecheck` — clean. - `pnpm exec vitest run cli/src/__tests__/open-url.test.ts cli/src/__tests__/auth-command-registration.test.ts` — pass. - Manual, in a headless container with no `xdg-open`: `pnpm paperclipai auth login -C <company-id>` now prints the approval URL and waits (previously crashed with `spawn xdg-open ENOENT`); `--no-browser` skips the open attempt; `PAPERCLIP_PUBLIC_URL=...` renders a reachable approval URL; completing approval in a browser stores the credential and `run list` works. ## Risks Low. `openUrl` became async; all four call sites updated to `await`. Desktop behavior is unchanged (successful spawn still resolves true and opens the browser). No API, schema, or migration changes. ## Model Used Claude Opus 4.8 (`claude-opus-4-8`) via Claude Code, with extended thinking and tool use, used to diagnose the bug and draft the fix and tests. Human-reviewed and tested on a live headless deployment. ## 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 - [x] I have not referenced internal/instance-local Paperclip issues or links - [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 — CLI only) - [ ] 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 - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |