mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
Devin FoleyandPaperclip
e5bf9d49a5
## Thinking Path > - Paperclip manages agents and records their task runs. > - Operators can enable Sentry reports for terminal run failures. > - A failed adapter can leave only the generic message “Adapter failed.” > - The run already records a process exit code and signal, but the report omits them. > - This pull request carries those two values through a strict capture boundary. > - Operators can distinguish a nonzero exit from signal termination when the message is generic. ## Linked Issues or Issue Description **What happened?** A failed run can store `exitCode: 1` or `signal: "SIGTERM"` while its Sentry event contains only `adapter_failed` and “Adapter failed.” The existing reporter drops both recorded fields. This occurs on the current master reporting path. **Expected behavior** The opt-in report preserves bounded process exit evidence without exporting adapter output or changing run behavior. **Steps to reproduce** Enable the backend Sentry DSN and report a failed run whose message is “Adapter failed” and whose stored signal is `SIGTERM`. Before this change, the event has no signal field. After this change, `run_failure.signal` is `SIGTERM` and the existing fingerprint stays the same. Related: #12105 and #8222 describe missing adapter/HTTP failure details. #13152 changes terminal-result cleanup classification, and #12886 adds process-failure classification and runtime URL checks. None forwards these stored fields through the Sentry reporter. This change does not resolve those broader issues. ## What Changed - Forward the stored exit code and signal from the terminal run reporter. - Accept only signed 32-bit integer exit codes; use `null` for missing or malformed values. - Accept only the reporting host's Node signal constants; use `null` for missing values and `unknown` for unrecognized values. - Keep the added fields in event-local context, outside tags and fingerprints. - Cover database-backed reporting, malformed input, privacy, and isolation through the real Sentry SDK. - Document the fields and their limits. - Give the dedicated Sentry job the normal PR dependency-resolution fallback, with lifecycle scripts disabled on every install and the required real-SDK test retained. ## Verification - Before the change: 16 report-shape/exit-field assertions failed in the focused capture suite. - After the change: 91 focused Sentry, DSN, and database-backed reporting tests passed. The real SDK uses an in-memory transport. - Final real-SDK test also passed with malformed metadata; it verifies that arbitrary signal text is absent from captured events and unrelated errors inherit no run context. - `pnpm -r typecheck`: passed. - `pnpm build`: passed. - `pnpm test:run`: exited nonzero after 718 general-server files: 14,085 tests passed, 14 failed, 86 skipped. Thirteen skill-service/cache failures reproduce on the unchanged base commit on this macOS host. One comment-wake test timed out; the complete 29-test suite passes on the unchanged base and in final-head Linux CI. Local isolated rechecks skipped because embedded PostgreSQL could not start; these are not counted as passes. The remaining local workspace/serialized lanes did not run after the failing first lane; all CI lanes passed. - Initial Sentry CI failed before tests with `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH`. Its install step lacked the normal PR fallback. The repaired real-SDK job passed. Security review then requested disabling lifecycle scripts for resolved dependencies; every install now uses `--ignore-scripts`. A fresh isolated checkout passed the exact script-disabled fallback and real-SDK contract. Final-head real-SDK CI and the security scan passed. - GitHub CI: all 54 checks passed on `37e0a836e50660f7753d367bcf5a4959eaf89b90`, including required `ci / verify` and `ci / e2e`; two unrelated checks skipped. - Greptile: 5/5 on that commit. No unresolved review comments. - Merge status: conflict-free; required CODEOWNER approval for the workflow change is still pending. ## Risks Low risk: this only adds two validated fields to existing opt-in error reports. It changes no database schema, run status, retry, fingerprint, or suppression rule. Process output and adapter result payloads remain excluded. A recorded signal does not identify its sender or prove an out-of-memory kill. Missing exit evidence stays unknown; this change does not establish the cause of a historical generic adapter failure. ## Model Used OpenAI GPT-6 (Codex), with reasoning, terminal tools, and code execution. The context window size is not exposed in this session. ## 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 references) - [x] My branch name describes the change and contains no internal ticket id or instance-derived details - [x] I have run focused tests locally and they pass; broader validation is recorded above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
70 lines
2.4 KiB
YAML
70 lines
2.4 KiB
YAML
name: Sentry SDK contract
|
|
|
|
on:
|
|
pull_request:
|
|
paths:
|
|
- .github/workflows/sentry-contract.yml
|
|
- server/package.json
|
|
- server/src/sentry*.ts
|
|
- server/src/peer-version-check.ts
|
|
- server/src/__tests__/*sentry*.test.ts
|
|
push:
|
|
branches: [master]
|
|
paths:
|
|
- .github/workflows/sentry-contract.yml
|
|
- server/package.json
|
|
- server/src/sentry*.ts
|
|
- server/src/peer-version-check.ts
|
|
- server/src/__tests__/*sentry*.test.ts
|
|
|
|
permissions:
|
|
contents: read
|
|
|
|
concurrency:
|
|
group: sentry-contract-${{ github.event.pull_request.number || github.ref }}
|
|
cancel-in-progress: true
|
|
|
|
jobs:
|
|
sentry-contract:
|
|
name: Real Sentry SDK isolation
|
|
runs-on: ubuntu-latest
|
|
timeout-minutes: 20
|
|
steps:
|
|
- name: Checkout
|
|
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
|
|
with:
|
|
persist-credentials: false
|
|
|
|
- name: Setup Node.js
|
|
uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7
|
|
with:
|
|
node-version: 24
|
|
package-manager-cache: false
|
|
|
|
- name: Setup pnpm
|
|
uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6
|
|
with:
|
|
version: 9.15.4
|
|
|
|
- name: Install workspace dependencies
|
|
# Master owns the lockfile; PR verification can start before its refresh.
|
|
# Resolve like the normal PR jobs, but this narrow contract needs no
|
|
# dependency or workspace lifecycle scripts, including on the fast path.
|
|
run: |
|
|
if ! pnpm install --frozen-lockfile --ignore-scripts; then
|
|
pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile
|
|
pnpm install --frozen-lockfile --ignore-scripts
|
|
fi
|
|
|
|
- name: Install the audited optional SDK outside the workspace
|
|
shell: bash
|
|
run: |
|
|
sentry_version=$(node -p 'require("./server/package.json").peerDependencies["@sentry/node"]')
|
|
npm install --prefix "$RUNNER_TEMP/sentry-contract-sdk" --ignore-scripts --no-audit --no-fund --package-lock=false "@sentry/node@$sentry_version"
|
|
|
|
- name: Verify run failure isolation with the real SDK
|
|
env:
|
|
NODE_PATH: ${{ runner.temp }}/sentry-contract-sdk/node_modules
|
|
PAPERCLIP_REQUIRE_SENTRY_TEST_SDK: "1"
|
|
run: pnpm --filter @paperclipai/server exec vitest run src/__tests__/run-failure-sentry-real-sdk.test.ts
|