2 Commits

Include renames

Author	SHA1	Message	Date
DottaandPaperclip	bd382eee53	fix(recovery): explain retained source verification failures (#17773) ... ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Workspace recovery must prove the identity of retained work before it uses an allocation. > - The verifier rejects several different contracts with the same `source_binding_invalid` code. > - A saved Retry can therefore stop before file preservation without enough evidence for a safe repair. > - Operators need a bounded observation of the selected source without another attempt or weaker guards. > - This pull request records closed failure reasons and adds an explicit read-only inspection on the run page. > - The benefit is a precise next diagnostic step while the saved work, intent, and ownership fences remain intact. ## Linked Issues or Issue Description Refs: #17741, #17053. This is a diagnostic follow-up. It does not change recovery admission or the retained-source acceptance policy. **What happened?** A retained-source recovery exhausted in `verify_execution` with only `source_binding_invalid`. Run and task diagnostics could not distinguish an invalid reference, lease policy, source binding, or reopened allocation. The normal recovery-actions read can also revalidate recovery state, so it was not a safe substitute for a current read-only inspection. **Expected behavior** Record an allowlisted reason at the rejected guard. Allow a reader to inspect one exact task/run/action without changing its attempts, saved input, receipts, or ownership. Clearly separate the recorded failure from current identity observations. **Steps to reproduce** 1. Prepare a terminal native run with a retained-source action and a lease policy of `reuse_by_environment`. 2. Attempt source verification. The guard must still reject that policy. 3. Open the failed run and choose Inspect recovery, then Inspect current source. The diagnostic should name `lease_policy_not_retained` and the allowlisted policy. ## What Changed - Split existing source-binding guards into closed reason codes. Keep each acceptance predicate and physical recovery action unchanged. - Persist validated, path-free reason/policy detail with future system recovery failures. Keep historical generic records compatible. - Add an explicit query on the existing task recovery-actions GET. Exact company/task/run/action selection bypasses read revalidation. Run selection lists at most 20 actions; inspection stops before per-lease lookup when a record has more than eight sources. - Add a manual inspection control on native export-failure run pages. Keep resolved retained actions inspectable. Show recorded failure and current recorded-source identity separately. - Update execution semantics and feature/access recipes. The shared type is the normalized server/UI contract; it contains no provider payload or secret. ## Verification - RED: the prior verifier failed both reason assertions for rejected warm policy and foreign-run lease binding. GREEN: the final focused source/HTTP, system-state, and UI suites pass 23 tests. The UI follow-up passes 8 tests after the review fixes. - HTTP tests prove exact selection, cross-company denial, resolved-action access, reused-allocation rejection, redaction, and unchanged actions, runs, leases, tasks, wakeup receipts, workspace operations, and activity. A nine-source case proves bounded inspection while the existing worker verifier still accepts all nine valid sources. - Review follow-up: show allowlisted saved detail beside a changed current observation, preserve all listed actions, and follow exact-action selection in verified/limited/error tests. The prior component fails the saved/current drift regression in an in-memory RED control; the current component passes. UI typecheck/build pass again. - `pnpm -r typecheck` and `pnpm build` pass. Token gates, maps, module boundaries, and `git diff --check` pass. - Rebased onto master `46de15ff4b` without conflicts. Range-diff proves both diagnostic commits are unchanged. On new head `a7e2bc87fcea75e76b6b736304d1287257f71174`, all 23 focused tests, full typecheck/build, token gates, maps, and module checks pass again. Both fresh reviews are 5/5 with zero unresolved threads. Final exact-head GitHub status is CLEAN: 55 success, 2 skipped, 1 neutral, no pending checks or failures. The [ci / verify aggregate](https://github.com/paperclipai/paperclip/actions/runs/40387002325/job/121802415824) passed in eight seconds. All test/build/typecheck/E2E lanes passed. - Existing full workspace recovery suite: 56/56 pass (103.5 seconds) before the byte-equivalent rebase. `pnpm test:run` hit repeated embedded PostgreSQL initialization failure before database-suite assertions and was stopped. No full local suite pass is claimed. The owned external `_test` database provided the focused HTTP/source verification. The prior browser attempt also stalled at Network.enable before its click; that attempt did not establish browser acceptance. - Current-head local browser acceptance passes through the actual run page: Inspect recovery lists the exact action, Inspect current source shows recorded generic failure separately from the current rejected policy, and reload resets inspection to the explicit control. Explicit selection works again after reload. The app used isolated data, an empty inherited environment, a paused synthetic agent, and no provider credentials. Final state remains failed/exhausted with zero attempts, zero wakeup receipts, zero workspace operations, and unchanged original generic failure. The local app and temporary tab were stopped. This proves the diagnostic journey, not the deployed source's actual lease policy or its recovery. - Final merge verification at `a7e2bc87fcea75e76b6b736304d1287257f71174`: GitHub reports CLEAN and mergeable. Both repository-required checks, `ci / verify` and `ci / e2e`, pass, as does the exact-head Greptile check (5/5, zero unresolved threads). The separate [Entire trail](https://entire.com/gh/paperclipai/paperclip/trails/7) has no recorded reviewer approval; no such approval is claimed. It is not a configured GitHub merge gate. - Initial local setup failures were distinct from product assertions: localhost sandbox EPERM required scoped local access; a browser harness import path was corrected. A test UUID type inference error and historical native-export UI selection gap were corrected before the green run. The live task remains blocked; this diagnostic does not claim it recovered. ## Risks - This change improves evidence. It does not prove provider termination, local descriptor integrity, or successful file preservation, and it does not repair the original source. - Current inspection spans multiple database reads. It is an observation, not a historical cause or execution authority. It performs no provider/filesystem calls and starts no retry or executor. - The optional persisted detail is backward compatible. Unknown values and extra fields are excluded. Oversized source lists remain uninspected through this explicit diagnostic path; physical recovery rules do not change. ## Model Used OpenAI Codex based on GPT-6, with code execution and browser tools. The exact serving model ID and context window were 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>	2026-10-27 08:21:28 -05:00
3 people	f3ca4d24bc	fix: repair dirty/foreign-branch execution worktrees (#9297) ... ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Execution workspaces are the bridge between Paperclip's control plane and a local agent's checked-out repository state. > - When a workspace is restored after a failed or interrupted run, the recorded branch can disagree with the branch currently checked out on disk. > - A clean branch mismatch can be reconciled safely, but a dirty mismatch needs a lossless path that does not discard uncommitted agent work. > - This pull request adds a quarantine-and-restore path that saves dirty work to a rescue branch, restores the recorded branch, and exposes the repair from the board UI and run page. > - The benefit is that operators can recover wedged execution workspaces without losing work or moving another live branch unexpectedly. ## Linked Issues or Issue Description No public GitHub issue exists for this workspace-recovery failure, so this PR includes the bug report inline. **What happened** A git worktree-backed execution workspace could become wedged when Paperclip expected one branch but found a different checked-out branch with dirty tracked or untracked files. The existing safe repair path refused the restore, leaving the source task blocked with no lossless one-click recovery path. **Expected behavior** Paperclip should preserve dirty work before restoring the recorded workspace branch. If another live workspace claims the checked-out branch, or an attached runtime service is active, the repair should refuse with clear operator-facing evidence instead of risking work loss or file contention. **Steps to reproduce** Create a git worktree execution workspace whose persisted branch name differs from the checked-out branch, add dirty tracked or untracked files in that worktree, then trigger workspace validation or use the branch reconcile endpoint. Before this change, the dirty mismatch remained blocked because Paperclip had no quarantine restore mode. **Paperclip version or commit** Observed on the pre-fix workspace-recovery implementation. Verified on this PR head after rebasing onto current `master`. **Deployment mode** Local trusted development/worktree deployments using git worktree execution workspaces and optional workspace runtime services. ## What Changed - Added dirty-worktree quarantine repair that creates a rescue branch, commits dirty tracked and untracked files there, restores the recorded branch, writes audit comments/activity, and preserves the live foreign branch ref. - Added `quarantine_restore` branch reconcile API support, recovery-action resolution, source-task wake behavior, execution-review preservation, claimant refusal, runtime-service refusal, and coverage for the non-transactional git ordering. - Added board UI controls for the repair action in the recovery card plus a compact failed-run workspace recovery surface that uses the same reconcile handlers. - Hardened Greptile follow-up cases by best-effort restoring the recorded branch after a mid-sequence rescue commit failure and by refusing quarantine restore while attached runtime services are active. ## Verification - `pnpm exec vitest run server/src/__tests__/execution-workspaces-service.test.ts -t "quarantine_restore"` - `pnpm exec vitest run server/src/__tests__/workspace-runtime.test.ts -t "workspace dirty quarantine branch repair"` - `pnpm exec vitest run server/src/__tests__/heartbeat-workspace-finalize-branch.test.ts -t "repairs clean unrecorded branch drift|adopts unrecorded forward branch drift"` - `pnpm exec vitest run server/src/__tests__/workspace-runtime-routes-authz.test.ts` - `pnpm --filter @paperclipai/server typecheck` - Earlier PR verification covered the route, service, heartbeat, UI component, and run-page recovery surfaces; Cutter posted public preview screenshots for the repair popover and run-page panel at https://github.com/paperclipai/paperclip/pull/9297#issuecomment-4926934211. - GitHub PR checks are green on `dcac76b05f4cf6e1ee16544c2831d83c7857e475`. - Greptile is 5/5 with zero annotations and no unresolved review threads on `dcac76b05f4cf6e1ee16544c2831d83c7857e475`. ## Risks - Moderate risk because the change intentionally runs git commands against local worktrees; the implementation refuses dirty repair when another claimant or active runtime service is detected and records rescue refs for auditability. - Compatibility / release-note callout for self-hosted operators: existing instances that left `enableWorkspaceBranchReconcileForward` unset now get automatic forward branch reconciliation during heartbeat workspace recovery. Operators who want the previous advisory-only behavior can set `experimental.enableWorkspaceBranchReconcileForward` to `false`; dirty quarantine repair can likewise be disabled with `experimental.enableWorkspaceDirtyQuarantineRepair: false`. - If the git rescue succeeds but a later database write fails, the worktree may already be restored while the recovery action remains open; this ordering is documented in code because git side effects cannot participate in the database transaction. - UI risk is limited to the workspace recovery surfaces and covered by component tests plus the existing Cutter visual preview. ## Model Used OpenAI Codex coding agent based on GPT-5, with repository tool use, shell execution, and local test execution. Earlier preserved commits on this branch also show Claude Code / Claude Opus 4.8 assistance in their commit 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] Branch naming exception documented: this PR preserves the existing worktree branch requested for publication while keeping the PR title and body public-facing - [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> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-09 18:19:49 -05:00