mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
76da2fec4feda6c6b9dd486f90069455ef5605a8
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
824b25654c |
fix(ui): keep a Retry for retained-file holds whose source cannot be verified (#19840)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - When a native run finishes but its files cannot be saved, the task stays blocked behind a retained-file recovery hold. > - The server records that hold as blocked and retryable, and the task notice tells the user to "Retry to try again". > - The task page hides its own Retry for these holds and relies on the retained-file recovery panel instead. > - That panel only binds the recovery record when a read-only source inspection returns `verified`. > - Inspection can return `blocked` for normal reasons: a sandbox that predates ownership receipts (state saved by an older release), or a workspace that moved on after its files were recovered. > - Then neither surface offers an action, and "Check file recovery status" always answers "File recovery changed. Refresh the task", also after a reload. > - This pull request binds the record by its own identity, uses inspection only to gate the physical file-recovery request, and keeps the task Retry visible whenever the panel has no control of its own. ## Linked Issues or Issue Description Refs #18848 (the change that added the retained-file recovery panel and hid the generic Retry for these holds). **What happened?** A blocked task with an active `native_workspace_sync_out_retry_exhausted` hold showed "The saved files need recovery before work can continue", "The recorded source is unavailable or has changed", and "Saving the agent's files failed after automatic retries. Retry to try again". It showed no Retry and no recover button. "Check file recovery status" always answered "File recovery changed. Refresh the task to inspect the current recovery", and a reload did not change it. The hold was saved by an earlier release: `systemRecovery` exhausted with `allocation_ownership_ambiguous` / `service_required`, and `workspaceRecovery` blocked with `retryable: true`. The source inspection endpoint returned `status: "blocked"` for it. A second task whose files were already recovered showed the same "need recovery" and "has changed" text, because its inspection reported a reference mismatch after recovery. **Expected behavior** A retryable hold always shows a working Retry or recover control. A notice that says "Retry" has a Retry button. Status checks never loop on "changed, refresh". Recovered files show as recovered. When the source cannot be verified, the page names the reason and who must inspect it. **Steps to reproduce** 1. Use an active retained-export hold whose `workspaceRecovery` is blocked and retryable, and whose `systemRecovery` is exhausted. 2. Make the source inspection (`GET /api/issues/:id/recovery-actions?inspectSourceRunId=…&inspectSourceActionId=…`) return `status: "blocked"`. 3. Open the task as a board operator. 4. Before this change, the page shows no Retry and no recover button. "Check file recovery status" reports "File recovery changed. Refresh the task". **Paperclip version or commit** Observed on build `8812c19`. The regressions fail on master `0f5ce5050e` before the production edits. ## What Changed - `WorkspaceExportRecovery`: bind the recovery record by action, run and schema identity, not by inspection status. Inspection status now gates only the physical "Recover saved files" request (and the request re-checks it before POST). - `WorkspaceExportRecovery`: report through `onRecoveryControlChange` whether the panel offers its own control. It reports `true` only while the source is loading or verified and the files are not yet recovered, or while it shows the previous-server stop confirmation. - `IssueDetail`: hide the task Retry for retained-export holds only while the panel reports that it owns the control. Otherwise the existing task Retry (`POST /issues/:id/retry`) shows. The server renews a bounded recovery generation and re-verifies the source. - Show recovered files as recovered even when inspection no longer matches after recovery. - Name an unverifiable source in plain words (with the code for other reasons). State that Paperclip does not apply files it can't verify. - Recovery text never asks a person to inspect a sandbox or environment, and never names an owner or administrator as the one who must act. This covers the retained-file panel, the run source inspector, the copyback wait, and the native cleanup quarantine message. The execution blocker and the recovery action read model describe a spent recovery budget from its current state, so text saved by older releases ("If Retry keeps failing, inspect the run and its environment") is not shown. - Task Retry uses the existing `createUuid()` helper, so it works on plain-HTTP previews without `crypto.randomUUID`. An accepted Retry also refreshes the inspected file-recovery state. After a failed accepted request, the unverified-source reason stays visible; it hides only while recovery runs. - When the recovery record was replaced, "Check file recovery status" reloads the task detail instead of telling the user to refresh. - Tests: component regressions for an old-release hold, a recovered hold with a mismatched source, verified ownership, and a replaced record. A page regression proves Retry is shown and calls `retryTask` for unverified and stale sources. A server safety test proves that Retry for the old-release hold shape is accepted, opens one new bounded generation, queues file recovery again, keeps the hold active, and is idempotent. ## Verification - RED (master `0f5ce5050e` production files, new tests): 5 component tests fail. The failure output shows the exact staging text: "Check file recovery status", "File recovery changed. Refresh the task to inspect the current recovery.", "The recorded source is unavailable or has changed." 2 page tests (unverified and stale) fail with `expected [ 'Check file recovery status', … ] to include 'Retry'`. - GREEN: `cd ui && npx vitest run src/components/WorkspaceExportRecovery.control.test.tsx src/components/WorkspaceExportRecovery.test.tsx src/components/RunWorkspaceRecoverySurface.test.tsx src/components/ExecutionBlockerNotice.test.tsx src/pages/IssueDetail.test.tsx` passes 320 tests. - Server: `cd server && npx vitest run src/services/task-retry.test.ts` passes 81 tests, including the new old-release safety case. This is safety evidence, not a red: the server path was already correct, and the defect was in the UI. - Full UI suite: 11564 pass. 4 tests in `IssuesList.test.tsx` and `CompanyEnvironments.test.tsx` failed under full-suite load and pass in isolation (106/106). They are unrelated to this change. - `pnpm check:token-gates` is clean. `node scripts/run-tsc.mjs -p ui/tsconfig.json --noEmit` reports no errors in the touched files. The remaining errors are local artifacts that were not built (plugin SDK UI entry, generated Storybook catalog). - Staging was read only (GET requests). The inspect request did not loop in a fresh tab: two calls in 30 seconds. Most of the tab's request volume is periodic company issue-list refetches, which this change does not touch. ## Risks - Low risk. This is a UI-only behavior change. Server authorization, source verification, stop receipts, and the bounded recovery policy are unchanged and remain authoritative. - The task Retry is now visible for retained-export holds whose source does not verify. Retry renews one recovery generation. The worker re-runs the same source-identity checks, so Retry cannot bypass an ownership refusal. If the refusal is permanent (for example an ambiguous sandbox allocation), the hold returns to exhausted and the page names the reason. An automatic fallback for such sources is follow-up work. - Wording: a parametrized UI test renders every recovery state and asserts that no administrator, owner, or "inspect the environment" copy appears. A server test proves that a stored old-release message is replaced by state-derived text. Both failed before the change. - Previous-release story: the old-release hold shape (board-owned, `systemRecovery` revision 5 exhausted with `service_required`, `workspaceRecovery` blocked and retryable) is covered by component, page, and server tests. No persisted state shape changes. > 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 - Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through Claude Code with tool use (repository search, file edits, shell, test execution, and a read-only browser). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
519b345d69 |
fix(ui): expose governed retained-file recovery (#18848)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The control plane must preserve saved work before it admits another attempt. > - Retained-file recovery already has an authorized API and a bounded worker. > - The task UI hid that operation when workspace repair blocked execution. > - The run inspector could check source identity but could not request preservation. > - This pull request exposes that existing operation with its exact source binding. > - The worker still owns file verification, stop receipts, and execution release. ## Linked Issues or Issue Description Refs #18678 and #18803 for the existing retained-source and continuation contracts. **What happened?** A failed native run could leave an active, system-owned export recovery action. The task showed a generic Retry control. Its workspace repair gate correctly refused another provider turn. The run inspector offered only read-only source inspection. The authorized file-preservation API had no production UI caller. **Expected behavior** An authorized board operator can request preservation of the exact retained files. The UI keeps the repair hold until the server records verified preservation. A lost response must keep the same request identity. **Steps to reproduce** 1. Use an active retained-export action whose workspace recovery is blocked and retryable. 2. Open its source task while `workspaceRepairRequired` is true. 3. Open the exact failed run and inspect recovery. 4. Before this change, neither surface can request the existing file-preservation operation. **Paperclip version or commit** The focused regression fails on master `1f55073f96b2485f49f98ee8c261aa14ec2f79ac` before the production UI edits. I checked the roadmap and searched open public PRs for this UI repair. No duplicate was found. This is a repair to the existing recovery capability. ## What Changed - Add Recover saved files to the existing task recovery area and exact-run inspector. Reuse the current board task-mutation access check and the existing `recoverWorkspace` API. - Read the exact action, run, company, task, and source observation again before sending. Refuse changed, retired, foreign, unverified, or non-retryable sources. - Save one account-scoped request before POST. Keep its key through a lost response and reload. Reconcile with GET before an explicit resend. Use a new key only after an explicit click and a fresh, confirmed exhausted attempt; do not resend automatically. - Show queued, recovering, blocked, and verified receipt states separately. A request acknowledgement does not mark files saved or resolve the task hold. Hide generic Retry for native export holds. Keep the existing Retry path for legacy restoration. - Cover both mounted surfaces and negative access, source, account, persistence, duplicate-click, and lost-response cases. Update the existing design guidance and correct the affected run-detail recipe only. ## Verification - RED: the new component regression fails with `missing Recover saved files` before production changes. The original failure is retained. - GREEN: 265 tests pass across `WorkspaceExportRecovery.control.test.tsx`, `WorkspaceExportRecovery.test.tsx`, `RunWorkspaceRecoverySurface.test.tsx`, and the full `IssueDetail.test.tsx` suite. - Review RED/GREEN: the prior UI reused the failed key after a confirmed exhausted attempt. A focused regression now proves a different key for an explicit new attempt, while the lost-response controls still prove key reuse. Mounted task controls prove that verified native preservation stays available, unverified or stale native sources never offer generic Retry, and viewers cannot mutate. A separate RED against the exact prior UI proves the lost legacy control. Its GREEN clicks the existing legacy Retry API while the workspace hold remains active. - Full UI TypeScript check passes: `node scripts/run-tsc.mjs -p ui/tsconfig.json --noEmit`. - `node scripts/check-token-gates.mjs`, `node scripts/check-maps.mjs`, and `git diff --check` pass. The map check proves structure only. - One early legacy test probe used an unsigned fixture and a readonly array; those setup failures are retained. The corrected same-user fixture proves the source RED and GREEN. The first test command used an unnamed project filter and stopped before loading tests. The corrected package configuration produced the recorded RED. One initial storage fault probe did not intercept jsdom's storage method; the corrected prototype fault proves that unavailable storage sends no POST. - Dependencies use Node 24.22.2 and a frozen pnpm 9.15.4 install with scripts disabled. The maintained shared and SDK prerequisite builds pass on this base. No dependency or lockfile changes are included. - A later Paperclip Review follow-up found the plain-HTTP UUID gap. Its focused regression fails before POST with the prior direct call, then passes with the existing UUID helper. The same storage, lost-response, retry and source controls remain green. - Original full CI `41715060582` fails two Cloud-auth task-entry browser checks at their existing heading assertion. The retained original artifacts show a late task pane (one already contains the heading; one still shows Loading conversation). The cause remains unproven. No auth, fixture, timeout or assertion change is included. Fresh Greptile is 5/5 with zero open threads and Paperclip Review passes on the UUID fix head. Current full CI `41720068249` attempt 1 failed. Three workers were terminated by AWS Spot capacity loss. The CLI startup and dense-ZIP checks also timed out at their unchanged deadlines. The CLI/shared input trees match a previous passing CI job, and the dense-ZIP case passes one focused local check. The CLI timeout cause remains unknown. The coordinator requested one bounded recheck. Attempt 2 succeeds on the same source, with all current checks green. No source, fixture, timeout or assertion changed for that recheck. The worker issued no retry. This pass does not establish the original CLI timeout cause. - Full recursive typecheck, repository tests, and application build were not run locally under the bounded UI verification scope. Full CI is requested. - No live preservation request, provider turn, deployment, or sandbox cleanup was performed for this repair. Original saved-file recovery remains unverified until rollout and real acceptance. ## Risks The client access check is a display guard. The existing server authorization and physical source, ownership, stop, and snapshot checks remain authoritative. Unavailable browser storage blocks the request before POST. A failed acknowledgement retains the request for its owner. The UI never calls task Retry, recovery resolution, reconstruction, provider execution, or destructive cleanup. The existing worker retains its bounded recovery policy and may finish the saved result after real preservation. ## Model Used OpenAI Codex, GPT-6 family, with repository inspection, code editing, and test execution. The exact runtime model ID and context-window size are not exposed to this agent. ## 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> |
||
|
|
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> |