diff --git a/cli/src/commands/client/issue.ts b/cli/src/commands/client/issue.ts index d7a5fb7b86..2e4cdf6ddc 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/DEVELOPING.md b/doc/DEVELOPING.md index c830de4bcb..bfede406df 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -238,6 +238,17 @@ owns committed updates. ## Hot-Restart Deploys +During a restart, the board's health, session, and access checks retry temporary +network/gateway failures and non-JSON API responses every five seconds. A new +page shows **Reconnecting to Paperclip** with a **Try again** action and waits +for startup health to become ready. Valid startup metadata remains available to +sign-in and invitation pages. An already +open page stays mounted during temporary background failures so unsaved edits +survive. Successful checks resume the same route and refresh other failed reads; +this recovery does not reload the browser or replay mutations. Authorization +failures still require sign-in or an explicit retry. Storybook **App / Connection +recovery** shows the startup recovery states. + Primary-instance rebuilds that restart `paperclip.service` can request one-shot live-run adoption instead of using the normal graceful shutdown drain. Before restarting the service, write the marker from the newly staged app with the current service PID: ```sh diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 12e000fea5..60105e3361 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 e69a6f1b8b..8840bff89e 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/doc/design/COMPONENT-INVENTORY.md b/doc/design/COMPONENT-INVENTORY.md index 91bf50d192..e9623703be 100644 --- a/doc/design/COMPONENT-INVENTORY.md +++ b/doc/design/COMPONENT-INVENTORY.md @@ -6,6 +6,13 @@ Run scope: `ui/src/components/` and `ui/src/pages/` on branch `design/token-extr ## Counts +### Connection recovery — 2026-09-29 + +`CloudAccessGate.tsx` contains the startup `CloudAccessError` composition, using +the existing Button and typography tokens. It covers automatic reconnection, +an in-flight check, and manual retry after access-check failures. See +`/design-guide` and Storybook **App / Connection recovery**. + ### Independent MCP connection setup — 2026-09-21 `ui/src/features/connections/remote-mcp/RemoteMcpConnectionSetup.tsx` is the controlled 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 ed2e39ed79..bfb9c8c609 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 ceb13379f7..edaa977e16 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 80ba78e3cf..1641d0ebee 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) | diff --git a/ui/src/App.test.tsx b/ui/src/App.test.tsx index 4730e3b782..23e55bd37e 100644 --- a/ui/src/App.test.tsx +++ b/ui/src/App.test.tsx @@ -121,7 +121,9 @@ describe("CloudAccessGate", () => { it("does not mistake a session service failure for a signed-out user", async () => { mockAuthApi.getSession.mockRejectedValue(new Error("Session service unavailable")); const root = renderGate(container); - await waitForText(container, "Session service unavailable"); + await waitForText(container, "Unable to load Paperclip"); + expect(container.querySelector("button")?.textContent).toBe("Try again"); + expect(container.textContent).not.toContain("Outlet content"); expect(container.textContent).not.toContain("Navigate:/auth"); expect(beginCloudSignInMock).not.toHaveBeenCalled(); unmountRoot(root); @@ -193,7 +195,8 @@ describe("CloudAccessGate", () => { mockAuthApi.getSession.mockResolvedValue({ user: { id: "invitee" } }); mockAccessApi.getCurrentBoardAccess.mockRejectedValueOnce(new Error("Access check unavailable")); const root = renderGate(container, true); - await waitForText(container, "Access check unavailable"); + await waitForText(container, "Unable to load Paperclip"); + expect(container.querySelector("button")?.textContent).toBe("Try again"); expect(container.textContent).not.toContain("Outlet content"); unmountRoot(root); }); diff --git a/ui/src/api/auth.ts b/ui/src/api/auth.ts index 0923f37ddf..2a66e77539 100644 --- a/ui/src/api/auth.ts +++ b/ui/src/api/auth.ts @@ -10,6 +10,7 @@ import { } from "@paperclipai/shared"; import { redactUrlSecrets } from "@/lib/redact-url-secrets"; import { tenantSessionRecovery } from "@/lib/tenant-session-recovery"; +import { readApiJson } from "./response"; type AuthErrorBody = | { @@ -158,13 +159,14 @@ export const authApi = { const res = await fetch("/api/auth/get-session", { credentials: "include", headers: { Accept: "application/json" }, + cache: "no-store", }); - const payload = await res.json().catch(() => null); + const payload = await readApiJson(res); if (!res.ok) { const recovery = tenantSessionRecovery.recoverIfNeeded(res.status, payload); if (recovery) return recovery; if (res.status === 401) return null; - throw new Error(`Failed to load session (${res.status})`); + throw extractAuthError(payload as AuthErrorBody, res.status); } const direct = toSession(payload); if (direct) return direct; diff --git a/ui/src/api/client.ts b/ui/src/api/client.ts index 88bb9a9cd1..1b4aab6f81 100644 --- a/ui/src/api/client.ts +++ b/ui/src/api/client.ts @@ -1,5 +1,6 @@ import { getPageVisibility, getVisibilityHeaderValue } from "@/lib/page-visibility"; import { tenantSessionRecovery } from "@/lib/tenant-session-recovery"; +import { readApiJson } from "./response"; const BASE = "/api"; @@ -56,7 +57,7 @@ async function request(path: string, init?: RequestInit): Promise { ...init, }); if (!res.ok) { - const errorBody = await res.json().catch(() => null); + const errorBody = await readApiJson(res); const recovery = tenantSessionRecovery.recoverIfNeeded(res.status, errorBody); if (recovery) return recovery; throw new ApiError( @@ -66,7 +67,7 @@ async function request(path: string, init?: RequestInit): Promise { ); } if (res.status === 204) return undefined as T; - return res.json(); + return readApiJson(res); } // --- In-tab request coalescing for identical safe GETs ----------------------- diff --git a/ui/src/api/health.ts b/ui/src/api/health.ts index df5c3b5031..2b6a2a08aa 100644 --- a/ui/src/api/health.ts +++ b/ui/src/api/health.ts @@ -1,5 +1,7 @@ import type { ServerInfoSnapshot } from "@paperclipai/shared"; import { tenantSessionRecovery } from "@/lib/tenant-session-recovery"; +import { ApiError } from "./client"; +import { ApiUnavailableError, readApiJson } from "./response"; export type DevServerHealthStatus = { enabled: true; @@ -24,7 +26,7 @@ export type CloudInstanceHealthStatus = { }; export type HealthStatus = { - status: "ok"; + status: "ok" | "starting"; version?: string; /** Commit of the running server; null when build metadata is unavailable. */ commit?: string | null; @@ -52,14 +54,17 @@ export const healthApi = { const res = await fetch("/api/health", { credentials: "include", headers: { Accept: "application/json" }, + cache: "no-store", }); + const payload = await readApiJson(res); if (!res.ok) { - const payload = await res.json().catch(() => null) as { error?: string } | null; const recovery = tenantSessionRecovery.recoverIfNeeded(res.status, payload); if (recovery) return recovery; - throw new Error(payload?.error ?? `Failed to load health (${res.status})`); + throw new ApiError(payload?.error ?? `Failed to load health (${res.status})`, res.status, payload); } - return res.json(); + // Startup recovery can still serve sign-in and deployment metadata. + if (payload?.status !== "ok" && payload?.status !== "starting") throw new ApiUnavailableError(res.status); + return payload; }, requestDevServerRestart: async (): Promise => { const res = await fetch("/api/health/dev-server/restart", { diff --git a/ui/src/api/response.test.ts b/ui/src/api/response.test.ts new file mode 100644 index 0000000000..b826ec2c22 --- /dev/null +++ b/ui/src/api/response.test.ts @@ -0,0 +1,94 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { api, ApiError } from "./client"; +import { authApi } from "./auth"; +import { healthApi } from "./health"; +import { ApiUnavailableError, isTemporaryApiError, readApiJson } from "./response"; + +afterEach(() => vi.unstubAllGlobals()); + +describe("API responses during a restart", () => { + it.each([200, 502, 503, 504])("handles HTML with status %i on every startup API", async (status) => { + const fetchMock = vi.fn().mockImplementation(async () => new Response("

Restarting

", { + status, + headers: { "Content-Type": "text/html" }, + })); + vi.stubGlobal("fetch", fetchMock); + + for (const read of [healthApi.get, authApi.getSession, () => api.get("/cli-auth/me")]) { + await expect(read()).rejects.toMatchObject({ + name: "ApiUnavailableError", + message: "Paperclip is temporarily unavailable. Please try again in a moment.", + status, + }); + } + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + it("does not replay a mutation after an HTML response", async () => { + const fetchMock = vi.fn().mockResolvedValue(new Response("", { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + await expect(api.post("/issues", { title: "Save once" })).rejects.toBeInstanceOf(ApiUnavailableError); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("preserves empty mutation successes", async () => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response(null, { status: 204 }))); + await expect(api.delete("/issues/example")).resolves.toBeUndefined(); + }); + + it("preserves valid startup health metadata for sign-in and invitations", async () => { + const payload = { + status: "starting", + deploymentMode: "authenticated", + cloud: { managed: true, managedBy: "paperclip-cloud", stackSlug: "example", cloudBaseUrl: "https://example.com" }, + }; + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(Response.json(payload))); + await expect(healthApi.get()).resolves.toEqual(payload); + }); + + it.each([null, {}, { status: "unhealthy" }])("rejects an unusable health payload: %j", async (payload) => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(Response.json(payload))); + await expect(healthApi.get()).rejects.toBeInstanceOf(ApiUnavailableError); + }); + + it.each([healthApi.get, authApi.getSession, () => api.get("/cli-auth/me")])( + "retains the status of JSON gateway errors for retry classification", + async (read) => { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(Response.json({ error: "Service unavailable" }, { status: 503 }))); + const error = await read().catch((error: unknown) => error); + expect(isTemporaryApiError(error)).toBe(true); + }, + ); + + it("preserves HTTP denials with or without JSON", async () => { + vi.stubGlobal("fetch", vi.fn() + .mockResolvedValueOnce(new Response("Forbidden", { status: 403 })) + .mockResolvedValueOnce(Response.json({ error: "Access denied" }, { status: 403 }))); + await expect(api.get("/denied")).rejects.toMatchObject({ name: "ApiError", status: 403 }); + await expect(api.get("/denied")).rejects.toMatchObject({ message: "Access denied", status: 403 }); + }); + + it("does not swallow an aborted response body", async () => { + const error = new DOMException("Aborted", "AbortError"); + const response = new Response(); + vi.spyOn(response, "json").mockRejectedValue(error); + await expect(readApiJson(response)).rejects.toBe(error); + }); + + it.each(["Failed to fetch", "NetworkError when attempting to fetch resource.", "Load failed"])( + "recognizes the browser network failure: %s", (message) => { + expect(isTemporaryApiError(new TypeError(message))).toBe(true); + }, + ); + + it.each([ + new ApiError("Unauthorized", 401, null), + new ApiError("Forbidden", 403, null), + new ApiError("Not found", 404, null), + new ApiError("Internal error", 500, null), + new TypeError("Cannot read properties of undefined"), + new DOMException("Aborted", "AbortError"), + ])("does not automatically retry non-transport errors: %s", (error) => { + expect(isTemporaryApiError(error)).toBe(false); + }); +}); diff --git a/ui/src/api/response.ts b/ui/src/api/response.ts new file mode 100644 index 0000000000..c491f30467 --- /dev/null +++ b/ui/src/api/response.ts @@ -0,0 +1,29 @@ +/** A proxy or restarting server answered an API request without usable JSON. */ +export class ApiUnavailableError extends Error { + constructor(public readonly status: number) { + super("Paperclip is temporarily unavailable. Please try again in a moment."); + this.name = "ApiUnavailableError"; + } +} + +export function isTemporaryApiError(error: unknown): boolean { + if (error instanceof ApiUnavailableError) return true; + if (!(error instanceof Error)) return false; + if ("status" in error && [502, 503, 504].includes(error.status as number)) return true; + // Fetch uses different network-failure messages across browsers. Do not + // classify arbitrary TypeErrors (programming bugs) or aborts as outages. + return error instanceof TypeError && /fetch|network|load failed/i.test(error.message); +} + +export async function readApiJson(response: Response): Promise { + try { + return await response.json() as T; + } catch (error) { + if (!(error instanceof SyntaxError)) throw error; + // An HTML fallback can have status 200 during a deployment. Never expose + // its parser error or treat it as a successful, empty API response. + if (response.ok || response.status >= 500) throw new ApiUnavailableError(response.status); + // Preserve HTTP/auth error handling even when a 4xx response has no JSON. + return null as T; + } +} diff --git a/ui/src/components/CloudAccessGate.test.tsx b/ui/src/components/CloudAccessGate.test.tsx new file mode 100644 index 0000000000..9d62e3f31d --- /dev/null +++ b/ui/src/components/CloudAccessGate.test.tsx @@ -0,0 +1,237 @@ +// @vitest-environment jsdom + +import { flushSync } from "react-dom"; +import { createRoot, type Root } from "react-dom/client"; +import { focusManager, QueryClient, QueryClientProvider, QueryObserver } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CloudAccessGate } from "./CloudAccessGate"; +import { queryKeys } from "@/lib/queryKeys"; +import { ApiUnavailableError } from "@/api/response"; + +vi.mock("@/lib/router", () => ({ + useLocation: () => ({ pathname: "/CON/agents/conan/instructions", search: "?tab=edit", hash: "#draft" }), + Navigate: ({ to }: { to: string }) =>
, + Outlet: () =>