From c9b93d7e8cc0474f7f97f581bcabe88addc27062 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 29 Sep 2026 08:47:33 -0500 Subject: [PATCH] fix: preserve terminal task owners during release (#14561) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Tasks record an assigned owner and separate checkout and execution locks. > - A completed task must retain its owner after execution ends. > - The release endpoint currently clears that owner when it clears the locks. > - This pull request preserves the assignee of Done and Cancelled tasks during release. > - Unfinished tasks keep the existing relinquishment behavior. > - The benefit is stable task attribution without retaining execution locks. ## Linked Issues or Issue Description **What happened?** An agent completed an assigned task, then called the release endpoint. The task stayed Done, but its assignee became null. The same defect affects Cancelled tasks. It caused the legacy Claude clarification/reuse and multiple-repository handoff E2E assertions to fail. **Expected behavior** Release must clear execution locks on terminal tasks and preserve their assignee, final status, and disposition timestamps. Release of unfinished tasks must still clear the agent assignee. Only In Progress work returns to Todo. **Steps to reproduce** 1. Create an assigned task with checkout and execution locks. 2. Complete or cancel the task. 3. Call `POST /api/issues/:id/release` as the assigned agent. 4. Read the saved task. Before this fix, its assignee is null. **Paperclip version or commit** Reproduced on master commit `d172197117a14b80a1eb2d2835a0e7cce2679656`. **Deployment mode** Local tests against real PostgreSQL through the production issue routes and services. This is a core lifecycle defect, independent of the agent adapter. Refs: #11689, #6899, #7769. These are related open release proposals. This is an independent fix limited to terminal task ownership. It does not include timer scheduling changes. ## What Changed - Preserve the current assignee when releasing Done or Cancelled tasks. - Keep all execution-lock cleanup and existing unfinished-task behavior. - Cover all seven task statuses through the release API and read back saved state. - Check disposition timestamps, activity attribution, and repeated board cleanup. - Update the API contract, agent reference, and CLI help. ## Verification - Red commit `ddaabb754`: the two terminal-owner regressions failed with `assigneeAgentId: null`; 12 other route tests passed. - Green: all 14 route tests pass, plus the existing successor-checkout race test (15 selected tests total). - Command: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/issue-stale-execution-lock-routes.test.ts src/__tests__/issues-service.test.ts -t 'stale issue execution lock routes|does not let stale release clobber a successor checkout lock'`. - The local host has exhausted its SysV semaphore pool. The red/green runs used the existing test-provider hook to start disposable Docker PostgreSQL 17 instances. Routes, services, migrations, and assertions were unchanged. No database tests in the selected set were skipped. The other 134 tests were excluded by the name filter. - Capability contract and inventory drift checks pass. - `pnpm build` and `pnpm -r typecheck` pass. - The local `pnpm test:run` was interrupted after environment failures while the complete sharded CI suite ran in parallel: native PostgreSQL bootstrap fails under the host semaphore limit, and the large Git fixture hits macOS `ENAMETOOLONG`. A focused rerun confirmed these happen before the relevant assertions. The interrupted local run is not counted as a full pass. - Greptile completed on `1caeeb827e9cb658ddb71f16c2421ec20f80634e` with **5/5**, a successful check, and no review threads. - All CI gates pass on the current head: typecheck, build, general and serialized tests, Runner checks, browser E2E, release packaging, and security checks. Server shard 11 passed on one targeted retry; the first attempt had 836 passing tests but an unhandled workspace-runtime startup rejection caused by an existing timing window. All other successful jobs were reused. - No paid provider evaluations were run. ## Risks - A caller that used release to erase ownership from terminal work will now retain that owner. An explicit assignment update or the board force-release option with `clearAssignee=true` can still clear it. - No schema or migration changes. The transaction, company access, assignee/run checks, and activity log remain in place. ## Model Used - OpenAI GPT-6 through Codex, with tool use, code execution, and test debugging. The session does not expose a more specific backend model version or context-window size. - OpenAI `gpt-6-luna` assisted with read-only test discovery and review. ## 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 --- cli/src/commands/client/issue.ts | 2 +- doc/SPEC-implementation.md | 6 +++ doc/SPEC.md | 5 +++ .../issue-stale-execution-lock-routes.test.ts | 40 +++++++++++++++---- server/src/services/issues.ts | 6 ++- skills/paperclip/references/api-reference.md | 8 ++-- 6 files changed, 53 insertions(+), 14 deletions(-) diff --git a/cli/src/commands/client/issue.ts b/cli/src/commands/client/issue.ts index d7a5fb7b866..2e4cdf6ddc0 100644 --- a/cli/src/commands/client/issue.ts +++ b/cli/src/commands/client/issue.ts @@ -1314,7 +1314,7 @@ export function registerIssueCommands(program: Command): void { addCommonClientOptions( issue .command("release") - .description("Release issue back to todo and clear assignee") + .description("Release issue execution locks; clear the assignee only for unfinished issues") .argument("", "Issue ID") .action(async (issueId: string, opts: BaseClientOptions) => { try { diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 12e000fea51..60105e3361e 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -1097,6 +1097,12 @@ Server behavior: 2. if updated row count is 0, return `409` with current owner/status 3. successful checkout sets `assignee_agent_id`, `status = in_progress`, and `started_at` +`POST /issues/:issueId/release` clears checkout and execution locks. For terminal +issues (`done` or `cancelled`), it preserves the assignee, status, and disposition +timestamps, including on repeated release. For unfinished issues it clears the +agent assignee; only `in_progress` changes to `todo`. Existing company access, +assignee/run ownership checks, and the `issue.released` activity event still apply. + `POST /issues/:issueId/admin/force-release` is an operator recovery endpoint for stale harness locks. It requires board access to the issue company, clears checkout and execution run lock fields, and may clear the agent assignee when `clearAssignee=true` is passed. The route must write an `issue.admin_force_release` activity log entry containing the previous checkout and execution run IDs. ## 10.5 Projects diff --git a/doc/SPEC.md b/doc/SPEC.md index e69a6f1b8b0..8840bff89ea 100644 --- a/doc/SPEC.md +++ b/doc/SPEC.md @@ -414,6 +414,11 @@ Tasks use **single assignment** (one agent per task) with **atomic checkout**: No optimistic locking or CRDTs needed. The single-assignment model + atomic checkout prevents conflicts at the design level. +Releasing a terminal task clears execution locks while preserving its assigned +owner and final status. Assignment remains part of the work history after Done +or Cancelled. Releasing unfinished work still relinquishes the agent assignment; +only an active `in_progress` task returns to `todo`. + ### Human in the Loop Agents can create tasks assigned to humans. The board member (or any human with access) can complete these tasks through the UI. diff --git a/server/src/__tests__/issue-stale-execution-lock-routes.test.ts b/server/src/__tests__/issue-stale-execution-lock-routes.test.ts index ed2e39ed79a..bfb9c8c6092 100644 --- a/server/src/__tests__/issue-stale-execution-lock-routes.test.ts +++ b/server/src/__tests__/issue-stale-execution-lock-routes.test.ts @@ -175,13 +175,16 @@ describeEmbeddedPostgres("stale issue execution lock routes", () => { }); it.each([ - { status: "done" as const, title: "Done release preserves status", completedAt: new Date() }, - { status: "cancelled" as const, title: "Cancelled release preserves status", cancelledAt: new Date() }, + { status: "done" as const, title: "Done release preserves owner", completedAt: new Date(), retainAssignee: true }, + { status: "cancelled" as const, title: "Cancelled release preserves owner", cancelledAt: new Date(), retainAssignee: true }, { status: "in_review" as const, title: "In review release preserves status" }, { status: "blocked" as const, title: "Blocked release preserves status" }, + { status: "backlog" as const, title: "Backlog release relinquishes ownership" }, + { status: "todo" as const, title: "Todo release relinquishes ownership" }, + { status: "in_progress" as const, title: "Active release requeues work", expectedStatus: "todo" }, ])( - "preserves $status when releasing a non-in_progress issue", - async ({ status, title, completedAt, cancelledAt }) => { + "$title and clears execution locks", + async ({ status, title, completedAt, cancelledAt, retainAssignee, expectedStatus = status }) => { const { companyId, agentId, currentRunId } = await seedCompanyAgentAndRuns(); const issueId = randomUUID(); await db.insert(issues).values({ @@ -204,7 +207,8 @@ describeEmbeddedPostgres("stale issue execution lock routes", () => { .send(); expect(res.status, JSON.stringify(res.body)).toBe(200); - expect(res.body.status).toBe(status); + const expectedAssignee = retainAssignee ? agentId : null; + expect(res.body).toMatchObject({ status: expectedStatus, assigneeAgentId: expectedAssignee }); const row = await db .select({ @@ -212,18 +216,40 @@ describeEmbeddedPostgres("stale issue execution lock routes", () => { assigneeAgentId: issues.assigneeAgentId, checkoutRunId: issues.checkoutRunId, executionRunId: issues.executionRunId, + executionAgentNameKey: issues.executionAgentNameKey, executionLockedAt: issues.executionLockedAt, + completedAt: issues.completedAt, + cancelledAt: issues.cancelledAt, }) .from(issues) .where(eq(issues.id, issueId)) .then((rows) => rows[0]); expect(row).toEqual({ - status, - assigneeAgentId: null, + status: expectedStatus, + assigneeAgentId: expectedAssignee, checkoutRunId: null, executionRunId: null, + executionAgentNameKey: null, executionLockedAt: null, + completedAt: completedAt ?? null, + cancelledAt: cancelledAt ?? null, }); + + const events = await db.select().from(activityLog).where(eq(activityLog.entityId, issueId)); + expect(events).toContainEqual(expect.objectContaining({ + action: "issue.released", companyId, agentId, runId: currentRunId, + })); + + if (retainAssignee) { + // Repeated cleanup, including by the board, must not erase attribution. + const retried = await request(createApp(boardActor(companyId))) + .post(`/api/issues/${issueId}/release`) + .send(); + expect(retried.status, JSON.stringify(retried.body)).toBe(200); + expect(retried.body).toMatchObject({ status, assigneeAgentId: agentId, checkoutRunId: null, executionRunId: null }); + const [saved] = await db.select().from(issues).where(eq(issues.id, issueId)); + expect(saved).toMatchObject(row); + } }, ); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index ceb13379f74..edaa977e16e 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -11787,14 +11787,16 @@ export function issueService(db: Db) { } } - // Release clears checkout/assignee locks; only in_progress work re-queues to todo. + // Terminal assignment records who owned the work, not a live execution + // claim. Cleanup must preserve it; unfinished release still relinquishes it. + const isTerminal = existing.status === "done" || existing.status === "cancelled"; const releaseStatus = existing.status === "in_progress" ? "todo" : existing.status; const updated = await tx .update(issues) .set({ status: releaseStatus, - assigneeAgentId: null, + assigneeAgentId: isTerminal ? existing.assigneeAgentId : null, checkoutRunId: null, executionRunId: null, executionAgentNameKey: null, diff --git a/skills/paperclip/references/api-reference.md b/skills/paperclip/references/api-reference.md index 80ba78e3cf3..1641d0ebee1 100644 --- a/skills/paperclip/references/api-reference.md +++ b/skills/paperclip/references/api-reference.md @@ -1355,9 +1355,9 @@ Terminal states: `done`, `cancelled` - `in_progress` = actively owned work. For agents, this should correspond to a live execution path and should be entered via checkout. - `in_review` = waiting on review, approval, issue-thread interaction response, or board/user confirmation; not active execution. - `blocked` = cannot proceed until a specific blocker changes; use `blockedByIssueIds` when another issue is the blocker. -- `done` = completed. -- `cancelled` = intentionally abandoned. -- `in_progress` requires an assignee (use checkout). +- `done` = completed. Release clears execution locks but preserves the assignee and `completedAt`. +- `cancelled` = intentionally abandoned. Release clears execution locks but preserves the assignee and `cancelledAt`. +- `in_progress` requires an assignee (use checkout). Release returns it to `todo` and clears the agent assignee. - `started_at` is auto-set on `in_progress`. - `completed_at` is auto-set on `done`. - One assignee per task at a time. @@ -1418,7 +1418,7 @@ Terminal states: `done`, `cancelled` | POST | `/api/companies/:companyId/issues` | Create issue (supports `blockedByIssueIds: string[]` for dependencies) | | PATCH | `/api/issues/:issueId` | Update issue; response is authoritative and includes `changes` + `comment` (`Prefer: return=minimal` supported); `blockedByIssueIds` replaces blocker set | | POST | `/api/issues/:issueId/checkout` | Atomic checkout (claim + start). Idempotent if you already own it. | -| POST | `/api/issues/:issueId/release` | Release task ownership | +| POST | `/api/issues/:issueId/release` | Release execution locks; preserve terminal task ownership | | GET | `/api/issues/:issueId/comments` | List comments | | GET | `/api/issues/:issueId/comments/:commentId` | Get a specific comment by ID | | POST | `/api/issues/:issueId/comments` | Add comment (@-mentions trigger wakeups) |