mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-13 04:02:28 +02:00
codex/codex-runner-default
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Devin FoleyandPaperclip
|
0bbd586a2f |
fix(server): wait for workspace restore before shutdown lease release (#15857)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Server shutdown must stop agent execution and preserve its files. > - Legacy adapters export workspace and instruction changes during settlement. > - Shutdown released their sandbox leases before this export finished. > - The provider closed command access, so the later Git export failed. > - This change stops an owned adapter through its cancellation control and waits for settlement before release. > - A bounded timeout retains the exact sandbox source behind the existing repair gate. ## Linked Issues or Issue Description **What happened?** A server restart interrupted a remote legacy run. Shutdown began releasing its sandbox lease while the adapter was still stopping its provider and restoring files. The sandbox provider closed command admission. The adapter's later Git export failed with `workspaceRestoreFailure: restore_failed`, leaving the task held for workspace repair. **Expected behavior** Shutdown keeps the exact lease available through provider stop, instruction collection, and workspace restore. Cleanup follows adapter settlement. A restore that exceeds the bounded deadline keeps its exact source for repair before later execution. **Steps to reproduce** 1. Start an owned legacy adapter with a sandbox workspace. 2. Send SIGTERM to the server while the adapter is active. 3. Delay adapter settlement until after the shutdown terminal write. 4. On the base branch, shutdown releases the lease without cancelling or joining the adapter. Its later export can encounter closed provider admission. Related: #14311 defines the provider-stop boundary for file collection. #15042 closes run admission during shutdown. This fix orders sandbox release after the adapter's existing file settlement. ## What Changed - Cancel each owned legacy adapter and join all affected runs within one shared 30-second window. - Commit a durable fence on the exact sandbox before exposing interruption. Other cleanup workers defer while export is pending. - Permit the exact aborted owner's stop-only request so shutdown does not block provider termination. - Reload current execution evidence after settlement. Release leases even when the adapter's ordinary terminal write wins the shutdown race. - Claim the current fence before recording timeout retention. An old snapshot cannot create a repair hold after successful settlement. - Recover fenced sources after controller loss, including released stop receipts. Block reuse or destruction of the physical allocation while file recovery is required, including direct resume and destroy calls with historical lease snapshots. Convert failed ordinary cleanup into stop-only recovery after its existing attempt settles. - Allow 90 seconds for shutdown in shipped Compose services. Preserve native runner restart ownership and unregistered process behavior. - Add database regressions for restore order, timeout, concurrent cancellation, another cleanup worker, controller loss, stale snapshots, stop-only settlement, and completion races. Document the shutdown order. ## Verification - The original ordering regression failed on the base implementation. - Latest recovery and environment runtime suites: 190 tests passed. The other four focused suites passed 136 tests, covering run control, shutdown, file repair, server startup, and environment runtime. - Server typecheck passed. Full `pnpm -r typecheck` and `pnpm build` passed on the latest head. - `pnpm test:run` was run locally. Its general-server group passed 18,559 tests and failed seven, with one unhandled error. The long run started before the review fixes, then loaded newer regression cases against older transformed modules. The current focused suites pass in a fresh process. The damaged-prior-epoch native-session-resume failure also reproduces on unchanged base commit |
||
|
Ross Sclafani
|
82ee0a68d4 |
chore(docker): pass PAPERCLIP_ALLOWED_HOSTNAMES through to quickstart (#6846)
## Thinking Path > - Operators run Paperclip in many places: localhost dev, LAN servers, Tailscale meshes, cloud VMs > - The server already supports a `PAPERCLIP_ALLOWED_HOSTNAMES` env var for hostname allow-listing (`server/src/config.ts`) > - But `docker/docker-compose.quickstart.yml` did not forward that env var from the host to the container > - So an operator running quickstart on a LAN gets "Hostname '<lan-ip>' is not allowed for this Paperclip instance" with no env-only escape hatch — they're forced to run the CLI inside the container to write `config.json` > - This PR adds a one-line passthrough so the existing env var works end-to-end with the quickstart compose file > - The benefit is parity with the server's documented config surface: anything settable via env on a bare-metal run is now settable via env on a quickstart docker run ## Linked Issues or Issue Description **What happened?** Running the quickstart compose file on a LAN host and opening the UI by the machine's LAN address fails with "Hostname '<lan-ip>' is not allowed for this Paperclip instance". The server supports `PAPERCLIP_ALLOWED_HOSTNAMES` for exactly this case and `doc/DOCKER.md` tells operators to set it, but `docker/docker-compose.quickstart.yml` never forwards the variable into the container, so setting it on the host has no effect. **Expected behavior** Setting `PAPERCLIP_ALLOWED_HOSTNAMES` on the host before `docker compose up` reaches the server, the same way `PAPERCLIP_PUBLIC_URL` and the provider keys do. **Steps to reproduce** 1. `export PAPERCLIP_ALLOWED_HOSTNAMES=my-lan-host` alongside the other quickstart variables. 2. `docker compose -f docker-compose.quickstart.yml up --build`. 3. Open `http://my-lan-host:3100` and observe the hostname rejection. **Paperclip version or commit** `master` when this PR was opened (May 2026); the quickstart file on current `master` still has no passthrough. The branch is rebased onto current `master`. **Deployment mode** Docker quickstart (`docker-compose.quickstart.yml`), authenticated and private. ## What Changed - `docker/docker-compose.quickstart.yml`: forward `PAPERCLIP_ALLOWED_HOSTNAMES` from the host environment with an empty default, matching the existing pattern used for `PAPERCLIP_PUBLIC_URL`, `OPENAI_API_KEY`, etc. ## Verification ```sh # 1. Set the env var echo \"PAPERCLIP_ALLOWED_HOSTNAMES=localhost,my-lan-ip\" >> .env # 2. Bring up the quickstart docker compose --env-file .env -f docker/docker-compose.quickstart.yml up -d # 3. Confirm the value reached the container docker compose -f docker/docker-compose.quickstart.yml exec paperclip \\ sh -c 'echo \"\$PAPERCLIP_ALLOWED_HOSTNAMES\"' # → localhost,my-lan-ip # 4. Confirm boot-time trusted-origins log includes the LAN host docker compose -f docker/docker-compose.quickstart.yml logs paperclip | grep trustedOrigins # 5. Confirm a request from the LAN host returns 401 (auth required), not the hostname rejection curl -i -H \"Host: my-lan-ip:3100\" http://localhost:3100/api/auth/get-session # → HTTP/1.1 401 Unauthorized ``` Tested locally on Linux with an authenticated/private deployment, migrated DB from another paperclip instance, and a LAN host reaching the container. The image was rebuilt with \`--no-cache\` from a clean checkout of this branch's tip (no other unmerged work in the build context) to confirm the change is self-contained. ## Risks Low risk. - Default value is empty string — behavior identical to before for any operator who doesn't set the var. - Env var name and semantics already implemented and documented on the server side (\`server/src/config.ts\`); this PR only routes the value through compose. - One-line yaml change, no code touched, no tests affected. ## Model Used - Claude (Anthropic) — Opus 4.7 (1M context). Used for the bug isolation, the env-var-vs-config-file choice, and the PR write-up. Authored alongside Ross Sclafani who tested end-to-end against a migrated LAN deployment. ## Checklist - [x] Thinking path traces from project context to this change - [x] Model used specified (with version + capability details) - [x] Checked ROADMAP.md — not a feature, no overlap with planned work - [x] Ran tests locally (\`pnpm install --frozen-lockfile\`, \`pnpm build\` clean; container rebuilt \`--no-cache\` from this branch tip and verified end-to-end) - Added or updated tests — N/A (compose env passthrough; no executable code path) - UI change screenshots — N/A (no UI) - [x] No documentation updates needed (env var already documented server-side) - [x] Considered risks (above) - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] Will address all Greptile/reviewer comments before requesting merge |
||
|
Zannis Kalampoukisandzannis
|
5db8ce3c44 |
fix(docker): make tini PID 1 in the server image so adopted orphans are reaped (#12137)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agent runs execute inside the server container, and they spawn many
short-lived descendants: git, the adapter CLI, esbuild, sh
> - The server image sets `ENTRYPOINT ["docker-entrypoint.sh"]`, and
that entrypoint ends in `exec`, so node becomes PID 1
> - Node reaps only the children it spawned itself. It installs no
`SIGCHLD`/`waitpid` handler for orphans that the kernel re-parents onto
PID 1, so those orphans stay as zombies forever
> - Zombies accumulate monotonically. When the cgroup pid limit is
reached, every `fork()` in the container fails and the instance is dead
> - This pull request installs `tini` and makes it PID 1 in front of the
existing entrypoint, adds a behavioural test that proves reaping, and
adds a `pids_limit` backstop to both compose files
> - The benefit is that a long-running container no longer degrades into
total fork failure, and a future regression is caught by CI instead of
by an outage
Depends-on: none — this change is self-contained in the image build and
its tests, and it touches no other in-flight branch
## Linked Issues or Issue Description
No public GitHub issue exists for this defect. It was found on a live
long-running instance. Description follows the bug report template.
**What happened?**
The server container ran for 22 hours and reached 2039 of 2048 pids in
its cgroup. Of 1760 processes, 1731 were zombies, and all 1731 had PID 1
as their parent. PID 1 was `node --import
./server/node_modules/tsx/dist/loader.mjs server/dist/index.js`. Zombies
accrued at about 79 per hour and were never reaped. The oldest zombie
was 20.8 hours old against a container uptime of 22.0 hours, so nothing
had been reaped since boot. Once the pid limit was reached, `git` and
`gh` failed with `pthread_create failed: Resource temporarily
unavailable`.
**Expected behavior**
PID 1 reaps orphaned processes that the kernel re-parents onto it. The
pid count of a long-running container stays flat instead of growing
without bound.
**Steps to reproduce**
1. Start the server image without `docker run --init` and without `init:
true`.
2. Run agent work that spawns descendants which outlive their immediate
parent.
3. Read `/sys/fs/cgroup/pids.current` and count processes in `Z` state
over several hours.
4. The zombie count grows monotonically and every zombie has PPID 1.
**Relevant logs or output**
```
cgroup pids.current / pids.max : 2039 / 2048
total processes : 1760
zombies : 1731 (98.4%)
parent of every zombie : PID 1 (1731/1731)
PID 1 cmdline : node --import .../tsx/dist/loader.mjs server/dist/index.js
container uptime : 22.0 h
oldest zombie : 20.8 h median: 14.4 h
zombie names : git 717, claude 280, MainThread 167, sleep 141,
esbuild 138, postgres 76, sh 65, sccache 50
```
**Additional context**
The fix pattern is already in this repository.
`docker/agent-runtime/Dockerfile.base` installs `tini` and sets
`ENTRYPOINT ["/usr/bin/tini", "--"]`. It was never applied to the server
image.
## What Changed
- `Dockerfile`: install `tini` in the `base` stage and set `ENTRYPOINT
["/usr/bin/tini", "--", "docker-entrypoint.sh"]`. The entrypoint stays
in the exec chain, so UID/GID remapping, `gosu`, and graceful shutdown
are unchanged.
- `scripts/assert-orphan-reaping.sh` (new): a behavioural probe. It
spawns a leader that forks a grandchild, exits the leader, and asserts
that the orphaned grandchild leaves `Z` state instead of persisting. It
fails closed if the grandchild is not re-parented onto PID 1, so a pass
cannot mean the check ran too early.
- `.github/workflows/docker.yml`: run that probe against the pushed
image after the publish step. The publish step is multi-arch with `push:
true`, so nothing is loaded into the runner daemon and the pushed tag is
the only thing to test. The cloud variant is `FROM production` and
inherits the same `ENTRYPOINT`.
- `scripts/docker-build-test.sh`: run the same probe against a local
build.
- `docker/docker-compose.yml` and
`docker/docker-compose.quickstart.yml`: add `pids_limit: 2048` as a
backstop, so a future leak dies visibly at its own ceiling instead of
starving the host of pids.
- `server/src/__tests__/container-init-reaping.test.ts` (new): 13
assertions that guard the configuration the probe depends on.
No per-orchestrator init lever was added. The image owning PID 1 covers
compose, plain `docker run`, the quadlet units, and the ECS task
definition in one place. Adding `init: true` in compose or
`initProcessEnabled` on the ECS task would nest a second init around
`tini`, and `tini` then warns on every boot that it is not PID 1. The
new test asserts the absence of both levers across all three manifests,
so the decision survives the next edit.
## Verification
| Check | Result |
|---|---|
| `scripts/assert-orphan-reaping.sh` against a real init | Grandchild
re-parented to PPID 1, then reaped. Exit 0. |
| Same probe forced against a genuine zombie | Reports `Z` and fails.
The failure branch is not vacuous. |
| Config guard against the pre-fix files | Exactly the 3 relevant
assertions turn red. |
| Config guard with `tini` removed from `apt-get` but the comments kept
| Red. It checks the install, not a mention of the name. |
| `cd server && npx vitest run
src/__tests__/container-init-reaping.test.ts` | 13 passed |
| `npx tsc --noEmit -p server` | Clean |
| `node scripts/check-docker-deps-stage.mjs` | PASS |
| `node --test scripts/release-verify-workflow.test.mjs` | 8 passed |
Not verified locally: no container runtime is available in the authoring
environment, so the probe has not run against a build of this image. The
new `docker.yml` step runs it against the pushed image on this PR.
## Risks
Low risk, but it is an image and entrypoint change, so it affects
deployments.
- `tini` adds one small package to the `base` stage.
`docker/agent-runtime/Dockerfile.base` already installs it from the same
Debian archive.
- Signal handling changes shape: `tini` receives `SIGTERM` and forwards
it to the entrypoint, which `exec`s node. `tini` forwards signals to its
direct child by default, and the exec chain keeps node as that child, so
graceful shutdown is preserved. A reviewer should confirm this on a real
stop.
- `pids_limit: 2048` is new for compose users. A deployment that
legitimately needs more than 2048 processes would now hit the ceiling.
The measured steady state on a busy instance was under 400.
- If a deployment already passes `--init` or `init: true`, `tini` runs
under another init and prints a warning that it is not PID 1. Reaping
still works because the outer init handles it. The compose files in this
repository do not set `init: true`.
## Model Used
Claude Opus 5 (`claude-opus-5`), extended thinking, with tool use and
code execution in an agent harness.
## 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 issues or links
- [x] My branch name describes the change and contains no internal
ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All 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: zannis <1011451+zannis@users.noreply.github.com>
|
||
|
Evyatar Bluzer
|
b287281940 |
docs: add inline comments to docker quickstart compose file (#2431)
## Problem The quickstart docker-compose file was recently moved to \`docker/docker-compose.quickstart.yml\` during the Docker reorganization but still have zero inline comments. When new user copy this file for self-hosting, they see variables like: - \`BETTER_AUTH_SECRET\` - what is this? How to generate? - \`PAPERCLIP_DEPLOYMENT_MODE: "authenticated"\` - what other modes available? - \`PAPERCLIP_DEPLOYMENT_EXPOSURE: "private"\` - what does private vs public mean? - \`OPENAI_API_KEY\` and \`ANTHROPIC_API_KEY\` - both required? Or just one? They have to go read DOCKER.md or other docs to understand each variable. ## What I changed Added inline YAML comments directly in the file: - Header block with step-by-step quickstart commands (cd docker, export, docker compose up) - Section headers grouping LLM keys, deployment settings, and secrets - Comment explaining each non-obvious variable with valid values - Note about \`BETTER_AUTH_SECRET\` with openssl generation command - Comment on the volume explaining what data it persist No functional change - only YAML comments added. |
||
|
Cody (Radius Red)andPaperclip
|
420cd4fd8d |
chore(docker): improve base image and organize docker files
- Add wget, ripgrep, python3, and GitHub CLI (gh) to base image - Add OPENCODE_ALLOW_ALL_MODELS=true to production ENV - Move compose files, onboard-smoke Dockerfile to docker/ - Move entrypoint script to scripts/docker-entrypoint.sh - Add Podman Quadlet unit files (pod, app, db containers) - Add docker/README.md with build, compose, and quadlet docs - Add scripts/docker-build-test.sh for local build validation - Update all doc references for new file locations - Keep main Dockerfile at project root (no .dockerignore changes needed) Co-Authored-By: Paperclip <noreply@paperclip.ing> |