mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
bc2d1efbe9bc74c19baca49c0f66cd401d78a09d
6704
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Dotta
|
fccad2fb5b | test(evals): exercise real card refresh failure and retained output | ||
|
Dotta
|
1fc2c961a2 | test(evals): include named profile prerequisite in inventory controls | ||
|
commitperclip[bot]
|
2d72f2c4c0 |
fix: Serialize ACPX session record saves per record to stop overlapping renames (EPERM on Windows) (#16925)
> [!NOTE] > Opened by Prospector's automated issue-fix pipeline. A maintainer reviews and decides; nothing here merges on its own. ## Thinking Path - Issue #15346 reports a defect in paperclipai/paperclip. - Root cause, as the fixing agent read it: ACPX saves one session record from several writers (live checkpoint timer, turn end, finalize, close) with no coordination; each save renames a temp file onto the same record file, and on Windows overlapping renames onto one file fail with EPERM. All of these saves go through Paperclip's session-store wrapper, which forwarded them concurrently. - This pull request: Serialize ACPX session record saves per record to stop overlapping renames (EPERM on Windows) ## What Changed - `packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.ts`: Every ACPX save in Paperclip passes through this wrapper, so it now puts all saves of one record id in a single queue (a promise chain shared by all wrappers in the process). Saves of one record never overlap and keep their order; a failed save still returns its own error and does not block the next one; saves of different records still run in parallel. ## Verification - Reproducing test(s), added in this PR: `packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.test.ts` - They fail on the base commit and pass with the fix, each twice in a fresh sandbox. - 3 independent attempts wrote 3 reproductions; this fix passes every one of them. - The repository's compile command passes with the fix applied. - 3 existing test file(s) near the change pass. - The full test suite passes except for the 246 file(s) that already fail on the base commit. ## Risks - Written by an automated pipeline, not a person: review it as you would an outside contribution. - Risk tier of the touched paths: 2. - The scope reviewer found it also changes, beyond the report: save() called when no earlier save of that record is pending - The scope reviewer found it also changes, beyond the report: persistedStore.save throwing synchronously - The scope reviewer found it also changes, beyond the report: Records with missing/undefined acpxRecordId - The scope reviewer found it also changes, beyond the report: save() for different record ids ## Model Used - opus, sonnet (Anthropic Claude, via Claude Code), writing and checking the change in an isolated sandbox. ## Linked Issues Fixes #15346 Other pull requests that name #15346: - #15370 (closed): fix(acpx): serialize session file saves and clean failed temporary writes ## 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) - [ ] 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 - [ ] 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 <!-- prospector:issue-fix v1 issue=15346 base=1c07b5903b1b report=4fe3fefc8864e00f --> Co-authored-by: commitperclip[bot] <284812584+commitperclip[bot]@users.noreply.github.com> |
||
|
Dotta
|
f2c3d07357 | test(evals): author named Notion read-only profile wizard | ||
|
|
585d37d4f2 |
[codex] Preserve AWS secret provider error classification (#10946)
## Thinking Path > - Paperclip is the open source control plane operators use to manage AI-agent companies. > - Operators can store managed secret values in AWS Secrets Manager provider vaults. > - The AWS JSON gateway already classifies safe provider failures such as access denial, expired credentials, throttling, and invalid requests. > - The outer provider wrapper was re-normalizing those typed failures, which collapsed an AWS access denial into a generic provider error before the service could produce actionable guidance. > - Missing runtime credentials also need to be recognized as an access problem so operators receive the correct recovery path. > - This pull request preserves typed provider errors, expands credential-related classification, and clarifies the safe credential/IAM remediation text. > - The benefit is that failed AWS secret creation tells operators whether to restore runtime credentials or grant the required capability without exposing AWS account or principal details. ## Linked Issues or Issue Description No public issue was found for this exact remaining server-side classification gap. Bug report: - What happened: an AWS Secrets Manager access-denied response could be classified correctly by the JSON gateway and then collapsed to `provider_error` by the outer provider wrapper; missing runtime credentials were also reported generically. - Expected behavior: safe typed AWS failures should retain their classification, and credential resolution failures should surface as access-denied guidance. - Steps to reproduce: configure an AWS provider vault with invalid or missing runtime credentials, or without `secretsmanager:CreateSecret`, then create a Paperclip-managed secret. - Paperclip version/commit: current `master` before this PR. - Deployment mode: Paperclip server with an AWS Secrets Manager provider vault. Related prior work: - Refs #9161 - Refs #9645 ## What Changed - Preserve `SecretProviderClientError` instances returned by the AWS JSON gateway instead of wrapping them as generic provider failures. - Classify missing, expired, invalid, or incorrectly signed AWS runtime credentials as access-denied failures. - Clarify sanitized remediation guidance to cover both credential resolution and `secretsmanager:CreateSecret` authorization. - Add regression coverage for gateway access-denied responses, missing runtime credentials, and the service-level actionable message. ## Verification - `pnpm exec vitest run server/src/__tests__/aws-secrets-manager-provider.test.ts server/src/__tests__/secrets-service.test.ts` — changed provider tests passed; the service file was skipped in the first no-DB focused run. - `pnpm --filter @paperclipai/server typecheck` — passed. - `pnpm -r typecheck` — passed after a frozen-lockfile dependency refresh. - `pnpm build` — passed. - `pnpm test:run` — 314 files / 3,443 tests passed; one unrelated existing test failed in `plugin-orchestration-apis.test.ts` because it observed `heartbeat.scheduling_suppressed` instead of `issue_commented`. The same failure reproduced in isolation and none of this PR's four changed files participate in that test. - `git diff --check public-gh/master...HEAD` — passed. ## Risks Low risk. The change affects only error classification and sanitized failure guidance for AWS Secrets Manager operations. Successful secret writes, schemas, migrations, API shapes, and stored secret material are unchanged. The regression tests assert safe classifications while keeping raw AWS account and principal details out of service responses. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5 family (exact runtime build and context-window size were not exposed), with reasoning, repository editing, shell execution, Git, GitHub, and test execution enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (not applicable: no documented contract changed) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: commitperclip[bot] <284812584+commitperclip[bot]@users.noreply.github.com> |
||
|
fengqing
|
05218571c3 |
fix(watchdogs): reject incomplete subtree scans (#15063)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task watchdogs verify stopped issue subtrees and restore valid work paths. > - A watchdog must read the full subtree before it can prove that all paths stopped. > - The current query stops after 100 levels and does not report omitted children. > - A running child beyond this limit can be absent from both review dispatch and mutation checks. > - This pull request reports an incomplete scan and blocks both decisions. > - Complete subtrees keep their current behavior and stop fingerprints. ## Linked Issues or Issue Description Fixes: #15061 Related PRs: #11327 changes subtree loading but keeps the same depth limit without a completeness result. #10331 changes mutation freshness after watchdog writes. Neither patch covers this boundary. This patch does not take code from those proposals. ## What Changed - Keep the existing 100-level traversal limit. - Check for an eligible child beyond the boundary in the same SQL statement as the subtree read. - Apply the same company, visibility, harness, and watchdog-origin filters to this check. - Return `incomplete` without a stop fingerprint when the scan omits eligible children. - Skip review dispatch and reject watchdog mutation revalidation for an incomplete scan. - Report incomplete scans through their own count and watched issue IDs. Log each newly incomplete issue at startup or during periodic reconciliation. Suppress unchanged results. Allow a new warning after the issue recovers and becomes incomplete again. - Add real PostgreSQL regressions for dispatch, a previously valid mutation scope, the complete boundary leaf, and excluded children. - Document the incomplete-scan behavior. ## Verification - Base: `569c7203aa24b95440682983ce7940ba1d4247bd`. Head: `06f68199656f979ef5eb4897851f0fbcbca592d3`. - Before the fix, two real PostgreSQL regressions fail. The service dispatches a review for a partial subtree. It also accepts a prior scope after a running depth-100 child appears. The other 19 scheduler checks pass. - On the exact final head, `pnpm -r typecheck` and `pnpm build` pass on isolated Ubuntu with Node 24.19.0, pnpm 9.15.4, and runner Rust 1.97.1. - All 75 related tests pass. Run `pnpm --filter @paperclipai/server exec vitest run src/__tests__/task-watchdogs-classifier.test.ts src/__tests__/task-watchdogs-scheduler.test.ts src/__tests__/server-startup-feedback-export.test.ts src/__tests__/issue-watchdogs-routes.test.ts --maxWorkers=1`. - Full `test:run` coverage passes across 29 logical groups: all official stable-runner lanes plus supplemental service and script tests. The selection covers all 900 server source test files without duplicate or omitted selections. It also includes UI, CLI, and other workspace projects. - A separate validation tree applies both this PR and #15067 to upstream `994d6edcdd4e15d5f9cc5cf8c135ac599104b86a` without conflicts. Type checks, the build, and all 98 related tests across five files pass. Both public PR heads remain unchanged. - Four full server lanes required test-process environment corrections. The host DNS search suffix resolved a `.invalid` fixture to an address. Ancestor project authentication settings affected hire fixtures. The old path-isolation tool also failed test imports with `EFAULT`. The exact base has the same failures: 8 failed and 128 passed assertions without isolation, then three failed imports with the old tool. All 136 checks pass on both the base and this head with the corrected isolation. All four original full lanes pass with the corrected tool, a resolver without a search suffix, empty ancestor project settings, and the standard Node path. No assertion or wait limit changed. The host configuration remains unchanged. - Earlier interrupted Ubuntu and Docker runs are retained and are not counted as passes. This exact-head Ubuntu run passes the original chat, port-conflict, and serialized lanes. The causes of the earlier host-loss and timing failures are not fully established. - The startup import regression in an earlier revision is fixed by a diagnostics module with no database dependencies. All 21 startup checks pass. - Exact-head CI, including the added scans after marking the PR ready: 54 successful checks and 2 skipped checks. Greptile: 5/5, no outstanding findings or follow-ups. Both review threads are resolved. - The final audit rechecked issue authors, assignees, comments, 206 open watchdog search results, root functions and fields, source history, and related diffs. The latest upstream is `994d6edcdd4e15d5f9cc5cf8c135ac599104b86a`. Its new patches do not cover this root. No competing root repair was found within the audited scope. The issue has no assignee or new comment. - No model provider, production database, or user instance was used. This verifies service behavior, not a full HTTP authorization bypass. ## Risks - A subtree with eligible children beyond 100 levels now waits instead of running a review. This is a deliberate safety boundary. It remains blocked until the scan can include the full subtree. - The boundary check adds a child-existence lookup to the existing recursive query. - No schema migration is required. Complete subtrees retain their fingerprint format. - This does not add unlimited traversal or close the separate race between a subtree snapshot and later source mutations. - The configuration lifecycle defect in #15060 is handled by separate PR #15067. This branch does not include its schema or configuration changes. ## Model Used - OpenAI Codex, based on GPT-6, assisted with source analysis, implementation, and test execution. The exact serving model ID and context window are not exposed in this session. The agent used reasoning, shell tools, GitHub CLI, and real PostgreSQL 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] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Dotta
|
49dbaf516b | test: cover card explanation after real source removal | ||
|
Andrii Furmanets
|
61ecc93033 |
fix: count a recovered shutdown interrupt as recovered (#15031)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The dashboard success rate counts recovered runs as successes
> - A run is recovered when a later retry in its chain succeeds
> - The summary puts `failed` and `timed_out` into recovered
> - An `interrupted` run, including `server_shutdown_interrupted`, falls
through to other
> - The success rate stays down after the retry succeeds
> - This pull request counts a recovered interrupt in recovered, not in
other
> - The benefit is that a shutdown that was retried successfully no
longer looks like a failed day
## Linked Issues or Issue Description
No open issue covers this. I searched open and closed issues and pull
requests.
**What happened?**
`dashboardService.summary` puts only `failed` and `timed_out` into the
recovered bucket. An `interrupted` run, including
`server_shutdown_interrupted`, goes to other even when `recovered` is
set. The success rate uses `succeeded + recovered`, so the day stays low
after a successful retry.
**Expected behavior**
When the recovered flag is set, count `interrupted` in recovered. An
interrupt with no successful retry stays in other.
**Steps to reproduce**
1. Record an interrupted run with error code
`server_shutdown_interrupted`.
2. Record a succeeded retry of that run.
3. Open the dashboard summary for that day.
4. Observe that the interrupt is in other and the success rate does not
include it.
**Paperclip version or commit**
master at
|
||
|
conreo
|
b46d1ab77b |
feat(ui): request a standalone window so the app can be installed (#13461)
## Thinking Path > - The Paperclip board is used from phones, and `ui/public/site.webmanifest` is what a browser reads when someone adds the board to a home screen. > - That manifest asks for `display: "browser"`. Chrome installs a web app only when `display` is `fullscreen`, `standalone` or `minimal-ui`; with `browser` it answers `manifest-display-not-supported` and adds a shortcut rather than an app, so every launch keeps the address bar. > - The user interface already implements the no-browser-chrome case. `StandaloneBrowserControls` renders Refresh, Share and Open in Browser, and it is rendered only when `mobile && isChromelessDisplayMode()`. `ui/src/lib/pwa-display-mode.ts` exists for no other purpose than to detect `standalone`, `fullscreen` and `window-controls-overlay`. > - None of those modes can occur on a mobile launch while the manifest says `browser`, so that component and its fallback controls are unreachable in practice. > - That makes the manifest value look like the outlier rather than the intent, so the change is one manifest field plus the test that pins it. ## Linked Issues or Issue Description Refs #2139 — "enh: Officially support Install as Progressive Web App", which asks for a supported home-screen install. This pull request is the installability half of that request; it does not touch notifications. **Subsystem affected** The web app manifest and the unit test that guards it: `ui/public/site.webmanifest` and `ui/src/lib/pwa-install-mode.test.ts`. No server, plugin, or database code. **Problem or motivation** Chrome's installability criteria require `display` to be one of `fullscreen`, `standalone` or `minimal-ui`. With `display: "browser"` Chrome reports `manifest-display-not-supported`, offers only a shortcut, and the launch keeps browser controls, so the board never behaves like an installed application even though the manifest, service worker, and icons all exist. The same value also makes existing code unreachable: the in-app replacement controls for a chromeless launch (`StandaloneBrowserControls`, gated on `mobile && isChromelessDisplayMode()`) cannot be triggered from a mobile home-screen launch, so the app carries a fallback that nothing can exercise. **Proposed solution** - Set `display: "standalone"` in `ui/public/site.webmanifest`. - Update `ui/src/lib/pwa-install-mode.test.ts`, which asserted `display === "browser"`. The assertion now pins `standalone` and a comment records why, so the next reader does not have to reconstruct this reasoning. The assertions that keep the legacy `mobile-web-app-capable` and the Apple metas out of `index.html` are unchanged. `standalone` rather than `fullscreen` for two reasons. It keeps the Android status bar showing the clock and battery, and it avoids a known Android behaviour of `fullscreen` launches: opening the soft keyboard triggers a fullscreen transition and a visible flicker ([Google issue 392925620](https://issuetracker.google.com/issues/392925620), [Stack Overflow 71606355](https://stackoverflow.com/questions/71606355/pwa-does-not-resize-window-when-virtual-keyboard-is-active-on-fullscreen-display)), which `standalone` does not do. **Alternatives considered** - `fullscreen`. Also installable, and it additionally hides the Android status bar. Rejected as the default for that reason; it is a one-word change if preferred. - Adding the legacy `mobile-web-app-capable` meta. Rejected: Chrome reads the manifest, so the meta is redundant. - Adding the Apple `apple-mobile-web-app-capable` metas for iOS. Rejected for this change: it is iOS-specific, and iOS is not the platform whose installability check this fixes. - Changing `orientation`, which #5564 reports. Rejected as out of scope; this pull request stays on `display` only. **Roadmap alignment** `ROADMAP.md` does not plan this work. Its only mention of install is the skills store. **Additional context** - #7792 attempted a standalone PWA earlier and was closed during triage. It bundled a deploy-pipeline change and a heartbeat change with the manifest change, and its title carried an internal ticket id. This pull request is deliberately the manifest change alone. - #5564 reports that the manifest's `orientation: any` ignores the Android rotation lock. Related, not addressed here. - The keyboard flicker above was reported against a board installed with `fullscreen`, which is one of the two reasons this pull request asks for `standalone` instead of the more immersive mode. ## What Changed - `ui/public/site.webmanifest`: `display` is now `"standalone"` instead of `"browser"`. - `ui/src/lib/pwa-install-mode.test.ts`: the expectation that pinned `browser` now pins `standalone`, with the reasoning in a comment; the meta assertions are unchanged. No runtime code, no dependency, and no data change. The icons the installability check also requires (`android-chrome-192x192.png`, `android-chrome-512x512.png`) are already present and unchanged. ## Verification - `vitest run src/lib/pwa-install-mode.test.ts src/lib/pwa-display-mode.test.ts` in `ui`: 2 files, 7 tests passed (the same 2 + 5 the sibling plugin-event change reports). - The manifest parses as JSON, keeps `display: "standalone"`, and keeps 192 and 512 pixel icons. - The only place in the repository that asserts the manifest's `display` value is the test updated here; the other references to `site.webmanifest` are request routing and HTTP log policy and are unaffected. - CI on the previous head of this branch had two failures unrelated to this change: an e2e hover-opacity assertion on a sidebar star button (`Star Zeta`: expected `1`, received `0`) and `TypeError: Cannot read properties of undefined (reading 'logicalEffectCount')` in the runner verify job. Both are in subsystems this change cannot reach; CI is re-running on this commit. ## Risks - Low. One manifest field and one test expectation; no runtime code changes. - The value applies to new installations. An app already installed with the previous value keeps its installed configuration until it is reinstalled, so nobody loses a working install. - iOS launches are unchanged: Safari does not take `display` from the manifest for home-screen launches, and this change does not add the Apple metas. - Nobody depends on the address bar being present in an installed launch; the in-app controls that replace it (`StandaloneBrowserControls`) are already implemented and become reachable. ## Model Used - Provider: DeepSeek. Model ID: `deepseek-v4-flash`. The model ran as a tool-using coding agent (shell, file edits, headless browser automation) inside a coding harness. The harness did not report a context-window size or a reasoning mode. ## 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 issues and linked them above (#2139, #7792, #5564) - [x] I have either (a) linked existing issues with `Fixes:` / `Closes` / `Refs #` OR (b) described the issue in-PR following the relevant issue template — both, see above - [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 ticket id or instance-derived details - [x] I have run the affected tests locally and they pass (2 files, 7 tests) - [x] I have added or updated tests where applicable — the guard test is updated, and it is the only test that pins this value - [x] I have updated relevant documentation to reflect my changes — no user-facing document describes the manifest; the updated test now records the reasoning in place - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green — CI is re-running on this commit after the unrelated failures described under Verification - [ ] Greptile is 5/5 — awaiting the review run on this commit - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
Dotta
|
c27280d1f0 | test(evals): author secure company secret and Morgan binding journey | ||
|
mdc2122andmdc2122
|
8153061d71 |
fix(pi-local): strip inherited server PAPERCLIP_* env from pi_local processes (#14503)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The `pi_local` adapter spawns the Pi CLI as a child process for
agent runs, environment tests, and model discovery
> - Each of these three paths builds the child env with `{
...process.env, ...env }`, so the child inherits the full server
environment
> - The server environment can hold server-only `PAPERCLIP_*` secrets
and config paths, such as `PAPERCLIP_AGENT_JWT_SECRET`,
`PAPERCLIP_TOOL_ACTION_SIGNING_SECRET`, and
`PAPERCLIP_SECRETS_MASTER_KEY_FILE`
> - `@paperclipai/adapter-utils` already has
`sanitizeInheritedPaperclipEnv`, and the shared `runChildProcess` path
uses it for its base env. `pi_local` passes its pre-merged env as
`opts.env`, which `runChildProcess` spreads over that sanitized base, so
the inherited server values come back
> - This pull request routes the three `pi_local` merges through
`sanitizeInheritedPaperclipEnv`
> - The benefit is that `pi_local` agent processes no longer receive
server secrets. This is a security hardening change.
## Linked Issues or Issue Description
No issue exists. This is a bug report in the in-PR format.
**What happened?**
A `pi_local` agent process receives the inherited `PAPERCLIP_*`
variables of the Paperclip server process. An env dump from inside a
`pi_local` run showed `PAPERCLIP_AGENT_JWT_SECRET`,
`PAPERCLIP_TOOL_ACTION_SIGNING_SECRET`,
`PAPERCLIP_SECRETS_MASTER_KEY_FILE`, and server config path variables.
The agent process can read these values.
**Expected behavior**
The `pi_local` child env drops inherited server `PAPERCLIP_*` variables,
the same as the shared `runChildProcess` path. Only
`PAPERCLIP_RUNTIME_API_URL`, `PAPERCLIP_LISTEN_HOST`, and
`PAPERCLIP_LISTEN_PORT` stay inherited. The run-scoped variables that
the adapter sets (for example `PAPERCLIP_AGENT_ID` and
`PAPERCLIP_API_KEY`) and the adapter-configured env still reach the
child.
**Steps to reproduce**
1. Start the Paperclip server with `PAPERCLIP_AGENT_JWT_SECRET` set in
its environment.
2. Run a `pi_local` agent whose command writes `process.env` to a file.
3. Read the file. `PAPERCLIP_AGENT_JWT_SECRET` is present.
**Agent adapter(s) involved**
`pi_local` only.
**Additional context**
Other CLI adapters appear to merge `process.env` into the child env in a
similar way (for example in some `execute.ts` and `test.ts` files). This
pull request does not change them. It keeps the scope to `pi_local`.
Related: #3030 and #12870 also touch inherited agent env. This pull
request is independent of #14479.
## What Changed
- `packages/adapters/pi-local/src/server/execute.ts`: the run env and
the runtime env that the adapter logs now start from
`sanitizeInheritedPaperclipEnv(process.env)` instead of `process.env`.
- `packages/adapters/pi-local/src/server/test.ts`: the environment test
env now starts from `sanitizeInheritedPaperclipEnv(process.env)`.
- `packages/adapters/pi-local/src/server/models.ts`: the model discovery
env (`pi --list-models`) now starts from
`sanitizeInheritedPaperclipEnv(process.env)`.
- `server/src/__tests__/pi-local-execute.test.ts`: add a regression
test. The test sets three server secrets on `process.env`, runs
`execute` with a fake Pi command that dumps its env, and checks that the
secrets are absent. It also checks that the adapter-configured env and
the run-scoped `PAPERCLIP_AGENT_ID` and `PAPERCLIP_API_KEY` are present.
## Verification
- `cd server && npx vitest run src/__tests__/pi-local-execute.test.ts
src/__tests__/pi-local-adapter-environment.test.ts
src/__tests__/pi-local-skill-sync.test.ts`: 7 tests pass.
- The new test fails without the source change (the secret assertion
fails) and passes with it.
- `npx vitest run packages/adapters/pi-local`: 42 tests pass.
- `pnpm --filter @paperclipai/adapter-pi-local typecheck`: pass.
- `cd server && tsc --noEmit` (after `pnpm --filter
@paperclipai/plugin-sdk ensure-build-deps`): pass.
- Follow-up coverage observes real child environments from direct model
discovery, agent execution, and `testEnvironment`. Each path omits
inherited secrets and retains the permitted runtime/listen variables and
configured env. Removing either the model-discovery or environment-probe
sanitizer independently makes the regression fail. All 49 scoped tests
and both direct typechecks pass.
## Risks
- Low risk. A `pi_local` setup that relied on an inherited server
`PAPERCLIP_*` variable (other than the three kept keys) no longer gets
it. Such a value can still be set in the adapter `env` config. This is
the same behavior as the shared `runChildProcess` 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 GPT-6.1 Sol, exact runtime model ID
`gpt6sol-cliproxy/gpt-6.1-sol`, with tool use and code execution.
Context-window and thinking-level metadata are not exposed. The prior
model and human-review disclosure was not substantiated by available
runtime metadata.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: mdc2122 <259970464+mdc2122@users.noreply.github.com>
|
||
|
Dotta
|
1ff71fdb8b | test: cover authenticated routine webhook with isolated human setup | ||
|
Dotta
|
994897f542 | test: retain selected input blockers and authenticate private oracle identity | ||
|
馨冉and馨冉
|
a02be1efc3 |
fix(continuation): degrade a stale interruptedRunId hint instead of failing the wake (#14807)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A wake builds its continuation envelope from a chain of resume-source run ids: explicit user resume, retry chain, previous run, then the interruptedRunId hint > - interruptedRunId is a best-effort hint written when a run is interrupted, and it is not cleared when an issue is reassigned to another task > - When the hint names another task's run (or a deleted row), the envelope build threw continuation_source_context_missing, so the new assignee's wake could never continue the work > - This pull request degrades a stale interruptedRunId hint to "no resume source" when it is the only source tier and does not resolve to this task, while explicit user resume and retry chains stay fail-closed > - The benefit is that cross-issue reassigns no longer brick an issue's continuation wakes ## Linked Issues or Issue Description Fixes #14723 ## What Changed - `server/src/services/execution-continuation.ts`: when `interruptedRunId` is the only resume-source tier and the referenced run is missing or belongs to another issue, continue without a resume source instead of throwing `continuation_source_context_missing` - The degraded envelope reports `trigger.sourceRunId: null` and no longer re-echoes the stale `interruptedRunId` into the new envelope (so the next wake does not inherit the poisoned hint) - Explicit user resume (`explicitUserContinuation.previousRunId`), `retryOfRunId`, and `previousRunId` tiers keep the existing fail-closed throw (`continuation_user_authorization_missing` for explicit user resume) - `server/src/services/execution-continuation.test.ts`: the "rejects handoff history from a different task" test is rewritten to the intended degraded behavior, plus a deleted-row case and a fail-closed control test for explicit user resume ## Verification - `cd server && npx vitest run src/services/execution-continuation.test.ts` — 35/35 pass (33 prior tests plus 2 new; 1 existing test rewritten to the new intended behavior) - `npx tsc --noEmit` reports no errors in the touched files - `git diff --check` is clean Manual reasoning check: the degrade branch only fires when `interruptedRunId` is the sole defined tier (`explicitUserSource`, `retryOfRunId`, `previousRunId` all null) and the run does not resolve to this task. Every other tier keeps the previous throw, covered by the new control test. ## Risks - Behavioral change: a wake that previously failed with `continuation_source_context_missing` for a stale interrupted-only hint now continues without resume history. This is the intended fix per the issue; the fail-closed invariant is preserved for all user-driven and retry source tiers - The `interruptedRunId` no longer propagates into the envelope when stale, so downstream consumers no longer see the stale id (they would previously only see it when the envelope build succeeded, which it never did for this case) ## Model Used Codex (OpenAI Codex CLI, GPT-5 based coding agent, repository-aware tool use, ~1M context window, extended reasoning), assisted by three read-only explorer sub-agents for issue triage and duplicate checking. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: 馨冉 <xinxincui239@gmail.com> |
||
|
Dotta
|
8c9b4ac273 | test(evals): author actual Notion catalog choice journey | ||
|
Dotta
|
509fcf3514 | test(evals): reject contradictory fresh preview success claims | ||
|
Dotta
|
25914805ac | test(evals): author real plugin preview tool journey | ||
|
Dotta
|
7733c54e21 | test(evals): verify rendered native artifacts without post-query direction | ||
|
Dotta
|
01e7e1b38a | test: bind scoped MCP cancellation source revision | ||
|
Dotta
|
b22ad6fc9c | test(evals): author supported native interruption with accepted fresh direction | ||
|
Dotta
|
5812962351 | test: cover genuine tracked accessibility skill revision update | ||
|
Dotta
|
73c414d968 | test: accept natural case closure and refusal phrasing | ||
|
Nicky Leach
|
ea727dc3c2 | Preserve plugin session continuity through warm execution and checkpoint recovery (#17820) | ||
|
Dotta
|
1b90c8d7c0 | test(evals): preserve immutable responses across ordinary-only queue actions | ||
|
Dotta
|
3cc814bdb4 | test(evals): add real scoped external assistant model companion | ||
|
DottaandPaperclip
|
dcfc2acfa8 |
Preserve complete bounded OpenCode MCP tool results (#17825)
<!-- entire-trail-link-start --> https://entire.com/gh/paperclipai/-/trails/34/changes/paperclip/34 <!-- entire-trail-link-end --> ## Thinking Path > - Paperclip manages AI agents and their governed tools. > - Native OpenCode receives tool results through a Runner-owned MCP bridge. > - The bridge cut serialized results at 64 KiB, even inside a JSON string. > - A tool could finish while its saved OpenCode output lost files and receipts. > - Native app results already use bounded results and saved-result pages. > - This change preserves complete JSON within a bounded output limit. > - Agents can read the result without repeating the completed action. ## Linked Issues or Issue Description **What happened?** The OpenCode MCP bridge sliced serialized JSON at 65,536 characters. A large file-list result became invalid JSON in OpenCode's saved output. The tool execution itself had completed. **Expected behavior** The bridge must deliver complete bounded JSON. A delivery limit must produce an explicit error, rather than a partial success value. A repeated call identity must not execute the action again. **Steps to reproduce** 1. Return a structured result with 27 file entries and duplicated text/data representations larger than 64 KiB. 2. Call the tool through the actual HTTP MCP bridge. 3. Parse the returned text as JSON. The old source fails at position 65,536. Related: #17740 changed assigned-tool catalog delivery. This PR repairs result delivery and does not change the tool catalog or authority. ## What Changed - Replace JSON slicing with complete serialization and a 1 MiB UTF-8 output limit. - Bound serialization traversal, including escaped strings and many small fields. - Keep the existing native 500 KB result/page and saved-artifact contracts. - Report oversized delivery with a warning that the action may already have completed. Keep call-identity replay on the original execution. - Add real HTTP regressions and document the transport limit. ## Verification - Red before fix: the large-result HTTP regression fails with an unterminated JSON string at position 65,536. - 91 focused MCP, OpenCode driver and continuation assertions pass on Node 24.22.1. They cover escaped UTF-8 pages, oversized delivery, authentication, cancellation and duplicate-call execution. - Workspace typecheck and build pass. - Actual native Rust → fixed MCP bridge → OpenCode 1.18.34 → `openrouter/deepseek/deepseek-v4.1-flash-1017` passed with a public synthetic 27-file result. One assigned read produced a 281,375-byte saved JSON file. The model used Bash/Python to parse all filenames and both unpredictable receipt fields. One finish completed the turn. - The native probe used a synthetic gateway and a mocked completion acknowledgment. It does not prove hosted deployment, board finalization or a particular upstream model vendor. - The full default test attempt is failed/incomplete. It was stopped after repeated embedded PostgreSQL bootstrap failures and skipped database suites. Its 15 warm-workspace cases pass unchanged on isolated PostgreSQL 18. A controlled default-database rerun fails before the test body when embedded PostgreSQL cannot start after five attempts. The test source is byte-identical to the base commit. The failed attempt remains recorded. - The final test-only commit checks a 500 KB escaped producer page inside the actual controller text envelope. The production bridge hash still matches the native proof. Its 91 focused tests and package TypeScript check pass. - Current-head verification (`73d18ca2`): 53 CI checks passed and two conditional Storybook checks skipped. Greptile is 5/5 on this exact head with no review threads. The failed/incomplete full local attempt and the controlled passing suites above remain separate evidence. ## Risks - Larger complete results use more bounded serialization memory than the old truncated output. - A bridge result above 1 MiB fails delivery. Native app results and saved-result pages are bounded at 500 KB before this bridge. The existing artifact protocol handles larger results. - A failed delivery cannot prove whether an upstream action took effect. The error forbids repeating it, and identical call identities retain the original execution. - This change does not alter model routing, admitted tools, credentials or trust policy. Hosted rollout and incident recovery remain separate verification steps. ## Model Used - OpenAI `gpt-6.1-sol`, with reasoning, tool use and code execution. The session did not expose an authoritative context-window size. Native acceptance used the exact DeepSeek model named above. ## 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 the relevant local tests and they pass; the full local-suite environment limitation is documented 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> |
||
|
DottaandPaperclip
|
db32bdf98b |
fix(github): recover review replay and preserve passive summaries (#17726)
<!-- entire-trail-link-start --> https://entire.com/gh/paperclipai/-/trails/8/changes/paperclip/8 <!-- entire-trail-link-end --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub tools save review assessments and publish their confirmed receipts. > - A timeout before publication staging can leave an invocation with no saved publication. > - Exact replay must repeat only the authorized preflight and converge on one publication. > - PR requests also share an App-owned summary comment. Progress and maintenance must preserve its final content. > - A native turn can intentionally wait for an author without sending another reply. > - This pull request repairs unstaged replay and prevents queued or passive-wait notices from replacing a published assessment. ## Linked Issues or Issue Description Related repairs: Refs #16883, Refs #16284, Refs #16275 recover saved publication receipts. Refs #17183 covers unavailable approved replay output. Refs #17714 fixes a separate publication lock-order problem. **What happened?** Exact review retries can return null `begin_review` output or `github_publication_unavailable` after an upstream read times out before publication staging. Separately, response maintenance can replace a completed PR assessment with “Review queued…” or a generic missing-response notice after an authorized no-new-work reply. The latter run can correctly commit a passive wait while its external summary is still overwritten. **Expected behavior** Exact retries repeat only the authorized preflight. A saved or uncertain publication keeps its existing recovery owner. Queued acknowledgements preserve final App-owned summary content. A committed native passive wait settles its current acknowledgement without writing a response or treating an old summary as a new-run reply. Success before presentation remains held until a durable decision exists; exact current-run confirmed receipts can retire their placeholders. Genuine missing/error responses retain fallback handling. **Steps to reproduce** 1. Hold a GitHub metadata read until an authorized gateway call times out before publication staging. Repeat the exact request. 2. Publish a valid PR assessment. Send an ordinary author reply that asks for no new work and no duplicate comment. 3. Commit the native `response_wake` with a no-comment server presentation decision. Run response maintenance twice. 4. The base returns the invalid replay result or replaces the assessment with progress/fallback text. **Paperclip version or commit** Replay regression: `41c42895fa862c586ff841f8c50200d93ffc83a4`. Maintenance regression: the previous PR head `c7553d2325df53d98d9febff7023ade4b1e704eb`. Both use real PostgreSQL and simulated GitHub responses. The maintenance failure was also observed in normal hosted use. **Deployment mode** Source checkout on macOS arm64 with isolated PostgreSQL 18, followed by a dedicated QA qualification. No production rollout is included. ## What Changed - Route exact `begin_review` replay through its typed owner. Repeat unstaged preflight only with the complete request digest and no successful invocation result. - Preserve current policy, immutable assessments, unique action keys, receipt recovery, and provider leases. - Preserve a verified App/marker-owned final shared PR comment during acknowledgements. New canonical assessments can still replace it at the same or a new head. - Project only a bounded server-committed native wait decision. Settle the current main-PR acknowledgement without provider writes or a fabricated current-run reply receipt. - Hold succeeded native runs while presentation is undecided, including the committed-before-presentation window. An exact current-run confirmed receipt can still retire its placeholder. - Keep missing/error fallback and uncertain publication checks. Update the bot guide, execution semantics, maps, and real-database regressions. ## Verification - **Causal RED:** exact begin replay returned null; exact unstaged submit replay returned unavailable. The new real-database maintenance test then reproduced the exact generic summary overwrite. Its first repair also exposed an earlier queued-acknowledgement overwrite. Both publication boundaries are repaired. Four additional ordering regressions reproduced the generic PATCH while native presentation was undecided; the corrected tests hold until the same row settles. - **Local GREEN:** 196 complete GitHub workflow cases pass on an empty migrated PostgreSQL fixture. This includes queued/running preservation, both success-before-presentation windows, null/scalar/invalid decisions, exact-current versus predecessor receipts, durable acknowledgement settlement, repeated no-op maintenance, eventual genuine missing/error fallback, pending writes and same/new-head canonical replacement. Workspace typecheck, exact-head build, maps and whitespace checks pass. The real database respects the actual selected-column projection. - **Command:** `PAPERCLIP_TEST_DATABASE_URL=<owned-postgres-18> pnpm --filter @paperclipai/server exec vitest run src/__tests__/chat-channels.integration.test.ts -t 'GitHub agent review workflow'`. - **CI/review:** historical heads `c7553d2325df53d98d9febff7023ade4b1e704eb` and `4d00d3af6f2ccbda8f38a98f81d392d82dd3c59c` had Greptile 5/5. Final readiness head `2d267f1e4aa0b835a11aede97798fbd800d9cd96` passed independent source review with no actionable finding and fresh Greptile 5/5 with zero unresolved threads; all 54 check runs pass with 2 intended skips; Snyk is successful and the merge state is CLEAN. The dedicated QA destination acceptance also passed. - **Full-suite limitation:** the earlier `pnpm test:run` attempt produced no final summary. Its retained log reports failures in unchanged workspace-runtime tests and 15-second gateway-audit and stale-question timeouts. Their cause remains unknown. Do not treat the targeted results as a full local-suite pass. - **Live evidence and limits:** normal native OpenCode/DeepSeek review delivery published a useful assessment, inline finding and App check at the exact reviewed head. A separate ordinary author reply delivered a useful answer and settled. Those are ordinary delivery proofs, not live injected-timeout proofs. A later no-new-work reply exposed the maintenance overwrite. The final combined source `070e9c4ead9341cd497b23f533a91bf194475e79` includes the exact publication repair and passed normal review → ordinary author reply → quiet destination acceptance. A normal current-head review restored the legitimate assessment; the ordinary no-new-work reply then committed a passive wait without new GitHub tools. The same 1,436-character summary stayed identical while queued/running, after terminal settlement, and more than 3 minutes 50 seconds after successful cleanup. Review rows and receipts stayed unchanged, with no owner, live run or queued input. The timeout fault fixtures prove replay and single-publication safety. The historical first upstream timeout producer remains unknown. The live board reads do not expose the raw acknowledgement action row: its terminal cancellation and next-pass no-op are real-PostgreSQL fixture proofs, while the hosted checks prove actual destination stability. The hosted journey did not inject a timeout or pause the terminal/presentation writer window. Private destinations, raw receipts and internal task identifiers remain outside this PR. ## Risks - Original and repeated metadata reads can overlap. The immutable assessment, action uniqueness and publication lease must remain authoritative. - Missing complete request identity or a deleted completed receipt remains held. The gateway does not reconstruct receipts or relax approval. - Passive suppression requires actual native mode/committed phase, current source ownership and the server's no-comment decision. It applies only to the main PR review surface. Uncertain current writes stay held; other surfaces keep normal placeholder retirement. - There is no migration, credential change, dependency or new retry scheduler. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`) in Codex assisted with diagnosis, implementation, tests, and source review. It used reasoning, repository 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 `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
208a093d19 |
fix(chat): preserve resumed review waits and retry newer saved input (#17725)
<!-- entire-trail-link-start --> https://entire.com/gh/paperclipai/-/trails/36/changes/paperclip/36 <!-- entire-trail-link-end --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub review agents publish findings and wait for an authorized reply or head change. > - Review tools could retain the requester after resume while completion lost that same wait proof. > - A yielded report could then queue its own continuation or repeat a missing-wait error. > - Ordinary Retry could also select an older failed turn while a newer saved request had no admission. > - This change shares the existing authority proofs and canonical admission owners. > - Current access, publication, execution and budget gates remain required. ## Linked Issues or Issue Description Refs #17318. Builds on the canonical saved-input retry owner in #17733. Related #16550 covers deferred legacy comments after ownership waits. The separate recovery repair in #17724 owns the cleanup-to-saved-input handoff; this PR does not add a second admission owner. A resumed review could publish an assessment, then turn its `response_wake` into an immediate `issue_status_changed` wake. A provider that omitted its continuation could instead repeat a completion error. A newer linked-human message saved during a budget hold could also remain unstarted because Retry selected the older failed source. ## What Changed - Resolve passive review waits through the accepted requester ledger used by GitHub tools. Require the latest reviewed head, exact company, task, agent, repository and processed publication under current access. - Fill omitted/null yielded continuation from that server proof. Permit an inherited normal tree resume only through its accepted parent identity. Permit an ordinary follow-up only after this run has a processed useful reply linked to its exact inbound message. Reject acknowledgements, partial assessments, pending or uncertain writes as that reply proof. - Recheck the same run/review/head/source proof during feedback, finalization and final status commit. Preserve explicit malformed-input rejection and Done policy. Add no monitor permission or publication grant. - Keep heartbeat and publication projections narrow. Preserve both published Copilot v62 identities and generate source-bound v63 from the pinned merged base. No v63 live qualification is claimed. - Certify the structured isolated-directory binding failure only at its pre-allocation boundary and only through canonical task Retry. Other workspace validation reasons and unknown effects remain denied. - Route Retry to eligible newer saved input through the existing canonical owner. Preserve the old result and skipped receipt. Require current principal, source, App, runtime fence, budget, native ownership and exact cleanup proof. - Recover absent historical App metadata only through an earlier trusted processed response receipt and an immutable non-null endpoint claim. Explicit malformed or conflicting metadata stays denied. - Revalidate execution through the exact claimed wake, coalesced task Retry and resolved user-continuation receipt after admission. A second normal Retry can recover the exact never-started authorization failure with unchanged saved input. Require empty execution evidence, exact diagnostics and no allocated or active executor. - Explain when a recorded result prevents exact replay. Ordinary board Retry directs a new task message to continue from saved work. Inherited requester identity alone never grants replay. - Keep accepted board-message preparation failures with the canonical task Retry owner. Certify the exact unchanged comment, original user wake, accepted same-user identity and absence of provider execution. Pin that source, recheck it at admission, and bind identity initialization to the coalesced intent and exact claimed successor receipt. Uncertifiable sources retain an actionable fresh-message refusal. - Return safe named authority categories for board Retry refusals. Update execution documentation and capability maps. Add no schema or polling loop. ## Verification - A fresh external review request exposed a separate isolated-directory preparation failure before allocation. Its first normal Retry saved an intent but admitted no successor. This follow-up certifies only that exact structured preparation boundary through the existing canonical Retry owner, with trusted failed settlement and no executor, lease, result or output. A real HTTP/queue regression reproduces the missing successor before the repair. Twenty-three current-source safety cases and the final 98-case Retry/HTTP group pass, including a fresh click after a denied intent, source revocation, accepted results and the older direct API remaining denied. The warning case uses actual configuration preparation and production event append; unknown warning shape or provider provenance stays denied. Full workspace typecheck, build, maps and diff checks pass. Renewed hosted normal Retry now succeeds on the final combined source: it preserves the exact saved request and linked requester, starts one native successor in a fresh attempt workspace, publishes the current-head 2/5 assessment and commits In review with passive response wake. - A later hosted normal Retry published a low-score assessment but lost its accepted source identity because the previous attempt failed before identity initialization. It repeated continuation errors, then submitted Done. This is failed acceptance, not a successful qualification. The initializer now retains the exact source only after execution-time failed-chat Retry authorization; generic Retry and Done policy are unchanged. A real queue/initializer regression fails with a null source on the previous code and passes with the newer saved source. Two real database completion cases verify that source yields the authoritative wait and commits In review. Renewed hosted normal Retry now retains the accepted source and exposes the authoritative current-review wait. It completed after two semantic feedback corrections (wrong contract revision, then wrong criterion IDs); the correct contract was present in emitted input. The third finish was accepted and committed In review, with no missing-wait loop or Done escalation. - Repeated board-message preparation failures now walk their canonical intent, failed dispatch and accepted parent identity, rechecking each contained predecessor. A production queue regression fails the second ordinary HTTP Retry before the repair and passes afterward. Changed source, contradictory execution evidence and cyclic receipts remain denied; traversal stops after twenty links. - New inherited omission cases failed before the repair. The final full PostgreSQL 18 wait/status suite passes **252 tests** under Node 24.22. It covers omitted/null input, malformed-first recovery, same-head resume, exact-source replies, changed heads, revoked access, pending/uncertain/mixed publications, final commit revalidation, no duplicate review and no self-wake. - **15 existing publication tests passed before the pinned merge** on a separate fresh database. They retain default partial-delivery and failure-fallback behavior. Context reads remain below the unchanged 16 KiB heartbeat and publication response limits with large saved output present. - Saved-input tests cover a real signed PR webhook, production accounting-budget refusal, normal policy recovery and canonical Retry. Historical producer-shaped tests prove the missing-App case red, then check trusted prior receipts and malformed, late, revoked or mismatched proof. A broader order-coupled test was reproduced on the prior source; its global fault injector now targets its own authorization query. The 43-case sequence passes without changing production behavior or its safety assertions. - New signed-ingress and real heartbeat execution tests reproduced the admitted-successor failure, then the next normal Retry failure. The final pinned merged source passes 249 full GitHub review/publication and Retry cases on a fresh database, including all 49 saved-input cases. A separate 74-case Retry/HTTP run also passes. Exact dispatch negatives cover source, company, agent, task, successor, receipt, pause, assignment, accepted success, controller and cleanup proof. Bootstrap contradictions remain denied. The canonical Retry suite passes 16 cases through its supported external PostgreSQL test connection. - The normal board HTTP route reproduced the unusable inherited-result refusal. It now returns actionable guidance without starting a successor. The actual UI Retry hook and banner tests pass 12 cases. - A production-shaped accepted board comment followed by a pre-provider qualification failure reproduced HTTP 409 before the repair. The final real HTTP/queue test certifies one canonical successor and preserves the accepted identity. It rejects changed source, actor, task, agent, result, controller, process, profile, allocation and receipt evidence. A post-acceptance source change keeps the same intent pending. Execution revalidation rejects removed or mismatched proof. The probe stops deliberately after production identity initialization; it does not claim provider execution. The final source passes the 75-case Retry group, the focused two-case board/inherited-policy group and 19 canonical Retry cases. Full workspace typecheck, build and map checks pass. - Full workspace `pnpm -r typecheck` and `pnpm build` pass. Map, module-boundary and diff checks pass. Full local `pnpm test:run` remains unverified. The 256-case explicit-native suite cannot initialize its packaged PostgreSQL because libzstd is absent; an existing command-scoped fallback did not resolve it. Current-head remote full-suite checks are required. Merged-source full GitHub+Retry (249), separate Retry/HTTP (74), full wait/status (252), profile/history and completion (75), and visible Retry hook/banner (12) checks pass. The fresh owned PostgreSQL 18 databases applied all 377 migrations. - Actual Rust Runner, OpenCode 1.18.34 and exact DeepSeek used unchanged production-generated synthetic contexts. Resume finished with an explicit server wait. The author-reply trial omitted continuation twice: its first report failed the existing empty-criteria check; the corrected second report still omitted continuation and received the exact server wait. It completed once with no missing-wait loop or Done escalation. Controller acknowledgments were synthetic; these probes do not establish hosted publication or board state. - Hosted combined testing proved automatic admission of the exact saved author reply, a useful GitHub answer and committed passive wait. It required two malformed-continuation rejections before a nullish fallback succeeded. A later normal Pause/Resume completed in one finish with explicit response_wake and no new review/comment. The admitted normal Retry then exposed the execution guard gap repaired here. Those remain checkpoint results. On the final combined source, renewed normal Retry completed actual native provider work and GitHub publication, committed the passive wait, released ownership and cleaned up its environment. A subsequent ordinary unmentioned author reply woke once, made no GitHub tool calls or new assessment, and completed its first finish with an explicit response wake. Both exact leases expired with successful cleanup and the queue was empty. However, final destination inspection found a separate automatic fallback overwrote the existing summary with a generic no-final-response message. External summary preservation therefore remains failed acceptance; the separate publication repair in #17726 owns that producer. Full combined publication preservation has not passed. These hosted runs include the pinned-base native additions; they do not qualify Copilot v63. - Current-head automated review is 5/5 with zero unresolved threads. All current-head CI gates passed (54 successful checks/contexts, four skipped, none pending). The earlier login-cleanup idle-work assertion also reproduced locally while its controlled baseline passed; its cause remains unknown, but the exact assertion passed in the fresh current-head CI shard. No timeout, assertion or production workaround was added. This PR is ready for review within its source, authority, native finalization, state, wake and cleanup scope. The independent automatic fallback publication regression is tracked by #17726; no complete combined workflow pass is claimed. ## Risks - The latest saved review defines the durable head boundary. Ingress must record a new head before admission. A reply receipt proves delivery only for its exact saved source; a shared GitHub comment ID alone cannot authorize this wait. - Historical App recovery cannot infer identity without direct proof or an earlier valid server response receipt. It never replays that response. - This does not infer review-score policy from instructions or override Done. No deployment, merge or repair of historical tasks is claimed by the PR. ## Model Used OpenAI `gpt-6.1-sol`, with reasoning, repository inspection, code editing and test 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 `#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 (focused database and native suites; full local suite limitation described 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 (verified on immutable latest head) - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (verified on the latest head) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
Dotta
|
1d62a20222 | test: reject contradictory case closure claims by clause | ||
|
DottaandPaperclip
|
f0877a9c7c |
fix: recover native work and requalify saved computers (#17724)
<!-- entire-trail-link-start --> https://entire.com/gh/paperclipai/-/trails/32/changes/paperclip/32 <!-- entire-trail-link-end --> ## Thinking Path > - Paperclip manages AI agents and preserves their work after an interruption. > - Native Stop must confirm termination and save the workspace. > - Task and run views must show the current recovery receipt. > - A new external reply can arrive while that cleanup is pending. > - The reply must return to its original chat owner after cleanup. > - A successful fresh retry also becomes the task’s bound workspace for its next ordinary turn. > - Managed computer updates can invalidate a saved snapshot qualification. > - This pull request fixes those instructions, acknowledgements, handoffs and the existing native-agent Test path. > - The operator can continue with new input while the stopped turn remains preserved. ## Linked Issues or Issue Description **What happened?** Stopped native runs could keep the initial workspace-repair instruction after files were recovered. Native Stop could also leave the shared cancellation state at requested after remote termination was confirmed. A new GitHub reply saved during cleanup could then remain queued because cleanup did not ask its original inbound owner to reconsider the receipt. After a managed snapshot update, an existing native agent could also need qualification while the generic environment Test required instance-admin access. **Expected behavior** Show the current saved-work state. Acknowledge Stop only after exact termination proof. Let an authorized operator use the existing saved-agent Run test to qualify its current computer. After cleanup, recheck the saved reply's current authority and admit it once as new input. Preserve the stopped run and uncertain external effects. **Steps to reproduce** 1. Stop an active native turn before it returns a semantic result. 2. Send a new authorized external reply while cleanup is pending. 3. Let workspace recovery and provider cleanup finish. Inspect the run instruction, Stop acknowledgement and queued reply. Related work: #17594 preserves native processes through controller outages. #17401 addresses a separate bootstrap failure. This pull request does not replace those fixes. ## What Changed - Use the exact run's workspace recovery receipt for task and run guidance, including the Activity ledger. - Retire an obsolete exact-session notice through the existing bounded reconciler. Preserve the separate no-replay hold. - Write the shared native Stop acknowledgement only after the audited Stop intent and every exact remote lease receipt match. - Delegate saved external input to the live inbound owner. Recheck its source, actor, endpoint, principal and conversation generation. Promote the same receipt in the transaction that creates its new run. - Use that owner from the cleanup callback and existing stranded-queue catch-up after a lost callback or restart. A denied source cannot abort other items. - Reuse the current private task directory after a successful fresh retry. Re-read the company/task/workspace binding and validate the canonical stable or attempt namespace without recreating missing files; keep a later fresh retry separate. - Extend the existing agent-scoped qualification owner to native agents. The clean saved-agent Test qualifies its exact computer before runtime testing. Reuse a current proof; require a real probe and verified cleanup for a stale proof. Preserve draft tests and metadata-only external/pool previews. - Update execution semantics, capability maps and the recovery verification recipe. Add a production-page Storybook interaction for the normal native Test. ## Verification - **Red:** persisted recovery states returned stale instructions; an exact terminated native lease left shared Stop requested; the real GitHub subscribed-message fixture left zero successors after cleanup. The controlled baseline restored the previous production source, reproduced the failure, then restored the fixed bytes with matching SHA-256 hashes. - **Green:** 382 durable-chat, native-continuation, Stop and recovery tests pass. The real GitHub/PostgreSQL handoff matrix passes 12/12. It covers callback, application restart, callback/sweep race, valid input behind a deleted source, and denial for unknown stop, wrong allocation, paused endpoint, revoked principal, deleted or changed source, changed conversation generation and foreign scope. Repeated callbacks preserve one receipt, its actor, one successor and the unchanged stopped predecessor. - **Workspace continuation red/green:** the real PostgreSQL preparation flow created a fresh attempt and saved a marker, then rejected the next ordinary turn with the same binding mismatch observed in deployed use. The repaired preparation and filesystem suites pass 86/86: stable and attempt-root reuse, saved-marker preservation, a later clean sibling, changed current bindings, foreign company/task, closed workspace, traversal, missing path and symlink refusal. No task row reset or old-turn replay is used. - **Qualification red/green:** a native saved-agent qualification returned 422, and its normal Test skipped qualification. The repaired path passes 224 qualification/immutable-snapshot/environment tests and 176 UI/upgrade tests. It covers revoked authority, foreign or changed computer, current-proof reuse, failed/missing proof, draft settings, settings changed during the probe, and external/pool previews. On exact current source, the real local Harness / Runtime UI also qualified a newly saved disposable Computer with missing proof through actual Daytona, then completed the actual OpenCode/OpenRouter hello probe using `openrouter/deepseek/deepseek-v4.1-flash-1017`. Supported availability changed from Legacy-only to Paperclip Runner after proof persistence. Qualification and runtime leases both completed normal release/cleanup. The five prior task run IDs were unchanged; no task wake was created. This is macOS source-server/UI with real Linux Daytona and provider services, not a hosted UI claim. Test probes are archived by the existing inspection contract; release does not claim physical deletion. - Full workspace `pnpm -r typecheck`, `pnpm build`, token, map and diff checks pass at `f6450f4739fc8141815b8b7c35fde22a655c5f90`. The broad local `pnpm test:run` passed 24,899 tests and reported one existing nested-clone test timeout at 15 seconds. The complete failing file then passed 21/21 with the normal timeout; the exact case also passed on the pinned earlier baseline. The broad runner stopped before its later groups. No timeout or production workaround was added. Prior head `f6450f4` has 53 successful checks, two conditional Storybook skips and Greptile 5/5 with no inline threads. New workspace-reuse head `fcb13399ec70f92f71bd8250081251dca3f48962` passes workspace typecheck, full build, 86 scoped tests, maps and diff checks; Current-head CI has 53 successful checks and two conditional Storybook skips; Greptile is 5/5 with no inline findings. Deployed ordinary-turn reuse acceptance passed as described below. - **Hosted Stop proof:** the real GitHub issue-mention path used native OpenCode, `openrouter/deepseek/deepseek-v4.1-flash-1017`, Daytona and an exact low-trust task boundary. Normal UI Stop interrupted an active Bash call. Native and shared acknowledgements matched the exact lease. Automatic recovery saved exactly `STARTED\n` (8 bytes), with no finish marker. Reload showed Stopped and saved work recovered. Only the original cancelled run remained. Normal five-minute cleanup completed its exact provider-destruction transition. - **Limits:** the provider buffered command output, so the marker was read after recovery. The approved local provider credential could not inspect the hosted sandbox; its 404 is not deletion proof. Cleanup success comes from the exact lease's platform destruction transition. An earlier hosted stale instruction disappeared after rollout with unchanged raw run/action data; that historical cause remains unexplained. Original incident holds and publication receipts were untouched. - **Hosted handoff proof:** after QA deployed the combined candidate containing `46b2e99`, startup admitted the previously saved GitHub author reply without another comment, Retry or manual action. The new native run used the exact original wake receipt and saved comment. It completed once, committed an external-response wait, preserved the stopped predecessor, emptied the queue and left the task In review. The answer appeared in the original GitHub conversation. Workspace finalization and the exact lease's normal destruction transition succeeded. - **Hosted workspace proof:** on the combined candidate containing `fcb13399`, normal Retry preserved the original authenticated GitHub request and completed a fresh native attempt. One later ordinary same-head author reply then completed in the exact same workspace ID and attempt directory, with no Retry/fresh directive. Its input tree digest matched the Retry's finalized digest; final copyback preserved that digest. The persisted assessment row and all five prior run records were unchanged, exactly one new reply run completed, and the task returned to In review with an empty queue and no live run. Both exact leases completed normal verified destruction. This used real native OpenCode, the exact DeepSeek/OpenRouter model and Daytona, without workspace-row repair or a new remote marker. - **Publication limit:** the workspace audit compares the persisted assessment and run history. The ordinary reply exposed a separate destination-summary fallback that changed the visible GitHub summary despite the preserved assessment row. The publication track is repairing that behavior; this proof does not claim the visible summary was preserved. - **Deployment boundary:** current-head CI/review and hosted same-source Retry/ordinary-workspace reuse acceptance passed. The separate visible-summary publication failure remains owned by the publication track. Combined hosted UI confirmation of the native-agent Test path is coordinated separately. Actual missing-proof UI/provider qualification has passed in the isolated local instance above; prior hosted Stop and same-input handoff proof remains separate. No merge or deployment is performed by this pull request. ## Risks Recovery state and termination receipts must match the exact source run and allocation. Missing or foreign proof keeps the hold. The chat owner must retain the original receipt and actor and recheck current authority. Cleanup cannot grant authority or replay the stopped turn. Existing native ownership, Stop, budget, permission and uncertain-effect guards remain in place. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`) through Codex, with reasoning, repository tools, source editing and test 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 `#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 (affected suites and isolated failing file pass; broad timeout retained 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 (current fcb has 53 successful checks/two conditional Storybook skips) - [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> |
||
|
Dotta
|
fcb04b4d0f | test(evals): author immutable answer delivery beside ordinary queue controls | ||
|
Dotta
|
3350d46798 | test: cover genuine terminal case closure and review gate | ||
|
Devin FoleyandPaperclip
|
832c44f26a |
fix: resume user follow-ups that reassign a task (#17821)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The execution control plane saves user messages while an earlier run needs recovery. > - A user comment can also assign a task back to its agent. > - That update creates an assignment wake, but continuation accepted only comment wake reasons. > - The saved question could stay blocked after workspace recovery and confirmed process cleanup. > - This pull request admits assignment wakes that carry a valid, undelivered user comment through the existing continuation checks. > - The same rule applies to new messages and saved requests already waiting for recovery. ## Linked Issues or Issue Description **What happened?** A human follow-up that also assigns a task back to its agent produces `issue_assigned`. After a native run stops during handoff, the continuation policy rejects that wake reason. The question remains saved behind the execution recovery hold even after the workspace is recovered and the old sandbox is destroyed. **Expected behavior** Deliver the saved human question once after the existing stop, ownership, authority, and execution gates pass. Assignment alone must not grant permission to continue. **Steps to reproduce** 1. Finish task work by assigning it to a human for review. Let the native run stop with `issue_reassigned` and retain its recovery hold. 2. Recover the workspace and confirm destruction of the old sandbox. 3. Post a new human question while assigning the task back to the agent. 4. Observe that the assignment wake stays deferred instead of delivering the question. Refs: #17288, #14156. These related changes cover legacy reassignment and assignments without comments. This change covers a human comment carried by an assignment wake behind a native or legacy continuation hold. ## What Changed - Share the comment-bearing wake classification between continuation admission and saved-request recovery. - Accept `issue_assigned` only through the existing persisted user-comment validation and execution gates. - Add database regressions for new and saved assignment wakes, concurrent recovery, single delivery, and unchanged prior run results. - Check that bare assignments, invalid authors and comments, changed ownership, active execution, and pending approvals or questions retain their holds. Cover native and legacy cleanup paths. - Update execution semantics and both recovery maps. ## Verification - Red: both new and saved assignment-with-comment cases stayed deferred without a successor on the original code. - Green: the final continuation suite passes all 284 tests. The four companion recovery, retry, and queue suites pass another 208 tests. - `pnpm -r typecheck` and `pnpm build` pass. - `git diff --check`, `node scripts/check-maps.mjs`, and `node scripts/check-module-boundaries.mjs` pass. - Apex review is 5/5 on `56c9c2d90dcda55b3f760946c9aba6aee448c361`, with no unresolved review threads. The only earlier finding concerned the contribution rule against new source comments; those comments were removed. - Full `pnpm test:run` is still running locally. CI is waiting for the `paperclip-public-pr-x64` runner fleet; the superseded workflow was cancelled so the current head could proceed. No passing CI claim is made yet. - Live recovery: corrected one existing saved wake to the supported comment reason. The normal server admitted one successor with the exact saved question and prior-run context, then started real tool calls. This verifies admission on the existing deployment; the new source change is not deployed yet. The resumed turn later failed with a separate provider terminal event whose error was null, followed by a workspace restore failure. The task is blocked again on that failure. Its useful answer has not been delivered, so end-to-end completion remains unverified. ## Risks - Assignment wakes now reach continuation validation when they carry a comment. The persisted comment must still belong to the task and requesting human, follow the stopped run, and remain undelivered. - The existing stop proof, cleanup, current assignment, approval, pause, budget, and competing-execution checks remain effective. Regression cases cover both native and legacy paths. - No schema migration or data rewrite is required for saved requests. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, source inspection, tool use, and code execution. The runtime does not expose the exact model ID or context window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and 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 - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>canary/v2026.1027.0-canary.3 |
||
|
Dotta
|
cf4aad60e0 | test(evals): author exhausted retry with real admission gate phases | ||
|
Dotta
|
01df721473 | test: add finite scoped external assistant catalog surface | ||
|
Dotta
|
0a97b24b64 | test(evals): author genuine owned native infrastructure recovery fixtures | ||
|
Dotta
|
02c0b64b72 | test(evals): disconnect exact Notion account through public UI | ||
|
Dotta
|
5e8eefb199 | test: preserve ordinary case handoff eligibility in facts fixture | ||
|
Dotta
|
3232b9d129 | test: cover attributed case diagnosis history | ||
|
Dotta
|
80a580212c | test(evals): verify Morgan-only Notion audience through actual picker | ||
|
DottaandPaperclip
|
4336b9ee02 |
fix(db): reserve transactions before checking pipeline capacity (#17831)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Interactive requests share a PostgreSQL connection pool with background work. > - Prepared queries can queue behind a slow query on a busy connection. > - Disabling pipelining lets queued work use the next free connection. > - The driver skips its transaction reservation callback when the pipeline limit is zero. > - This pull request runs that callback before the capacity check. > - Operators can test a pool without pipelining while transactions retain their connection ownership. ## Linked Issues or Issue Description **What happened?** With `PGMAX_PIPELINE=0`, postgres.js 3.4.9 skips `onexecute` in its short-circuit capacity check. A transaction then lacks its reserved connection. Real PostgreSQL reproduces `UNSAFE_TRANSACTION` through the app client and an undefined connection error through the raw driver. This prevents safe use of the existing setting to reduce query queue delays. **Expected behavior** A transaction must reserve its connection regardless of pipeline capacity. Commit, rollback, savepoints, and disconnect recovery must preserve their existing contracts. **Steps to reproduce** 1. Use the pre-change driver patch from `e61ee4b005f8e13f64af15f69cb62305dd869f82`. 2. Start disposable PostgreSQL and run the `no-pipeline` scenario in `packages/db/src/__fixtures__/postgres-connection-recovery.mjs` with `TEST_DATABASE_URL`, `DRIVER_FORMAT=esm`, and `RECOVERY_CASE=no-pipeline`. 3. Observe the transaction failure. Apply this patch and repeat with both `esm` and `cjs`. Refs #13643, which introduced the existing connection recovery patch. Searched related open and closed driver PRs. This change extends that patch and preserves its disconnect safeguards. ## What Changed - Move the transaction reservation callback before the pipeline capacity check in both driver distributions. - Add real PostgreSQL regressions for commits, concurrent transaction reads, isolation, rollback, savepoints, reserved connections, and progress on a freed pool connection. - Document the existing `PGMAX_PIPELINE=0` setting and its driver prerequisite. Keep the default pipeline limit unchanged. ## Verification - `pnpm -r typecheck`: passed. - `pnpm build`: passed. - Real PostgreSQL 18.6 in an owned, disposable Docker container: the old patch fails the new transaction scenario in both ESM and CommonJS. The corrected patch passes all 18 scenarios, including all 12 existing disconnect recovery cases. The container was removed afterward. - Local service benchmark: actual `createDb` with prepared statements, two warmed connections, two controlled advisory lock waits, and the complete three-query `getDefaultCompanyGoal` path. Two warmups and five alternating measured samples per setting. Pipeline 100: median 413.44 ms, range 413.33–416.10 ms. Pipeline 0 with the patch: median 12.22 ms, range 11.86–12.57 ms. Complete SQL, bindings, and results match. Five of five reads finish while the unrelated slow query remains blocked. These are synthetic local queue measurements, not hosted latency claims. - App client compatibility on real PostgreSQL: commit, concurrent transaction reads, rollback, nested savepoint rollback, dedicated connections, and liveness connections passed. - `pnpm test:run`: attempted, then stopped after repeated local embedded PostgreSQL startup failures caused by the host semaphore limit. This is an incomplete local full-suite run, not a pass. The focused driver checks above used disposable Docker PostgreSQL instead. The initial sandbox attempt also stopped before test execution because tsx could not bind its IPC socket. Final CI results will be recorded here. - Current head `72039f71f63530300bf241475d42e857297dfb90`: Greptile and Paperclip review are both 5/5. All three earlier findings are resolved. [CI run 40431567516](https://github.com/paperclipai/paperclip/actions/runs/40431567516) ran the complete validation matrix. The database lane passed all 265 tests, including all 18 real PostgreSQL driver regressions. One cleanup timer case timed out in server shard 6/12; its isolated real-PostgreSQL reproduction passed once and failed once, while five runs with the pre-change driver passed. That evidence is retained and does not establish the timer failure's cause. A single CI retry of the failed shard passed. The final aggregate `ci / verify` gate is waiting for a runner. Its first queued request did not start for more than 14 minutes. A guarded queue reset confirmed that it was the only unfinished job and that every test job had passed; attempt 3 restarts only that gate. No test result was replaced or bypassed. The required gate must pass before merge. ## Risks - This changes a shared driver callback order. Both module formats and existing disconnect recovery are covered by real database tests. - This PR does not change pool size, enable the setting by default, retry SQL, or change schemas. Disabling pipelining may reduce batch throughput. Operators must compare the workload before retaining that setting. - The local queue benchmark proves the mechanism. Hosted request improvement still needs acceptance after the patched image is deployed. ## Model Used OpenAI GPT-6 in Codex, with code execution and browser developer tools. The exact serving model identifier and context window are 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 `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [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> |
||
|
Dotta
|
fdd772c7a7 | test: cover corrected case facts and concurrent reread | ||
|
Dotta
|
89163d7f73 | test(evals): author governed independent native review journey | ||
|
Dotta
|
6d04ec8d5f | test(evals): rotate gateway tokens through isolated real UI flow | ||
|
Dotta
|
bcd7ca5e77 | test(evals): distinguish substantive blue steering and observed branch | ||
|
Dotta
|
1ad14e1abc | docs(evals): pin pending handoff cutoff to committed active steering | ||
|
Dotta
|
77b5eadea1 | docs(evals): record finite pending authoring and privacy handoffs | ||
|
Dotta
|
ac8ac79d68 | test(evals): author actual native same-turn steering journey | ||
|
Dotta
|
37b72e28e3 | test(evals): provision genuine plugin-managed routine and preserve its controls | ||
|
Dotta
|
edbf426db3 | test(evals): distinguish unsupported skill action from unfinished denial coverage |