mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
DottaandPaperclip
818d2bb661
## Thinking Path > - Paperclip manages AI agents and their work. > - Native execution persists a result before it completes cleanup and workspace export. > - A later execution error enters the failure-write path. > - That write must reject an already selected result. The existing code reports lease loss for every rejected write. > - This can hide the category of the first error even when the same controller still owns an unexpired lease. > - This change adds closed diagnostic labels after exact ownership and accepted-result checks. It keeps the existing control error and recovery policy. ## Linked Issues or Issue Description **What happened?** A local real-PostgreSQL regression accepts a result through the production control plane, then injects a harmless filesystem failure. The production executor reports `native_session_lease_lost`. It loses the original failure category because the failure transition requires a null result ID. **Expected behavior** Keep the result guard and the existing lifecycle classification. Record a closed diagnostic category when the selected result is accepted and the controller still has exact, unexpired authority. Keep ordinary lease-loss fencing when that proof is absent or its diagnostic lock is busy. **Steps to reproduce** Run the new post-result cases in `server/src/services/heartbeat/runtime-execution.test.ts`. They use the real executor, control-plane result commit, heartbeat recovery path and PostgreSQL. The provider boundary is controlled. No live provider is used. **Paperclip version or commit** The regression fails on base `7d1e9b3477`. Related open PR #16917 changes legacy lease recovery. Open #19063 adds terminal finalizer reasons, and #15310 preserves database finalization errors. They do not change this executor diagnostic branch. This change only adds native diagnostic evidence and does not reclaim a lease. ## What Changed - Add a typed diagnostic with a closed stage, category and accepted-result flag. Do not retain the source message, path, arbitrary code or lease token. - After a rejected failure write, check the selected accepted result and the exact company, task, run, owner, attempt, generation, process identity and expiry. Use a coordinator-only `NOWAIT` lock. Fall back to the original control error if the diagnostic read fails. - Report the closed event after rollback. Keep `native_session_lease_lost` as the error message and leave every failure-write predicate unchanged. - Add real-PostgreSQL safety cases for stale and expired authority, owner replacement, Stop state, result selection, pending accounting, one provider invocation, closed labels and diagnostic sink failure. - Document the closed event, unchanged authority, and possible late diagnostic sink completion. - Add a competing-row-lock barrier. It verifies actual PostgreSQL lock refusal and control settlement before the competing lock is released. ## Verification - Permanent regression on unchanged base: RED because the closed diagnostic is absent. - NOWAIT mutation: RED when only NOWAIT is removed. PostgreSQL reports lock timeout instead of immediate lock refusal. The test uses barriers and the actual database error, with no elapsed-time assertion. - Final related suites: 56 tests passed across runtime execution and provider failure integration tests. - Full server source and test typecheck: passed. - Full `pnpm -r typecheck`: passed in an independent locked-dependency checkout. - Full `pnpm build`: passed. - Full `pnpm test:run`: running in the independent immutable checkout. All six chat shards passed; the remaining suites are still pending. - The first current-head CI attempt failed in two cases: workspace runtime initial fixture HTTP readiness and instructions-bundle error-message rendering. Both exact cases passed in isolation on unchanged PR head and base. The original causes remain unknown after bounded test-only observation. One controlled same-head rerun of each failed job passed. Current required CI and all other completed checks passed. Original failures remain recorded; no infrastructure or flaky-test claim is made. - Module boundary check and `git diff --check`: passed. - Exact-head Greptile: 5/5 with no actionable findings or open review threads. Automatic native review remains queued. That advisory check is not required by current master rules; no current-head native assessment is claimed. - Earlier fixture setup failures remain recorded. These included missing fixture fields and observing the Drizzle wrapper instead of its PostgreSQL cause. They are not the regression RED. An earlier broad typecheck was interrupted for the required NOWAIT revision; the final full typecheck passed. - No hosted export recovery or original hosted failure cause is claimed. ## Risks The extra read is diagnostic only. It uses NOWAIT and cannot wait for a competing coordinator lock. Its failure preserves the existing control classification. The event sink uses the existing 60-second execution-control bound. A slow sink can finish later and emit a late event. Result selection, Stop, accounting, retry eligibility and provider work do not depend on event success. No schema or lifecycle contract changes are required. ## Model Used OpenAI GPT-6.1, model ID `gpt-6.1-sol`, with reasoning, tool use and code execution. The host did not expose the context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>