mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 14:16:39 +01:00
## Thinking Path > - Paperclip manages agents and tasks. > - Plugins contribute views to the task sidebar. > - A ready view can belong to an earlier task. > - Runner records activity under its authenticated run. > - This PR maps that activity to installed task-panel declarations. > - The sidebar checks readiness after current-run use and restores use after reconnect. ## Linked Issues or Issue Description Builds on merged contract #18129 and Runner publisher #18050. This PR targets master and is ready for review. Earlier combined verification is retained in [the verification archive](https://github.com/paperclipai/paperclip/pull/18050#issuecomment-6489369676). **Subsystem affected** Run-event reads, plugin task contributions and the task sidebar. **Problem or motivation** Readiness polling can show a view left ready by another task. Reload and reconnect must recover current-run use without losing concurrent live activity. **Proposed solution** Map recorded activity to installed task-panel declarations. Merge authorized history and live activity, then perform bounded readiness checks for the running agent. Preserve the existing read-only view. ## What Changed - Add optional `{ namespace, activityKey }` activation mappings to task-panel declarations. Legacy declarations remain valid but contribute no task panel. - Add `view=plugin-panels` to the existing run-events route. Preserve run-read and telemetry-read authorization, secret redaction and private-run denial. Map activity to installed contributions and return at most 128 panel identities. - Scope activation caches to current panel versions and activity declarations. Preserve unchanged panels during unrelated plugin updates and history reads. - Merge history and live hints without losing concurrent activations. Filter by company, task, run and agent. Restore history on mount and reconnect. - Replace periodic availability checks with up to three readiness attempts after activity. Fresh activity restarts settled transient failures. Pending checks, successful readiness and HTTP 4xx failures are left alone. - Reuse live run updates with the panel cache key. Recover only panel history after a pending subscription read. Mark expected not-ready query results so the global error reporter skips them; real failures remain reportable. - Preserve current-run dismissal and the read-only plugin view. Update manifest comments and producer/consumer documentation. ## Verification - Current head: `5e04d757c`, based on master `09c05c550`. Range comparison confirmed all six feature commits were unchanged by the rebase. The final follow-up keys readiness by the plugin installation ID and availability path. - `cd ui && node ../node_modules/vitest/vitest.mjs run src/hooks/useTaskPluginPanels.test.tsx` — all 45 cases passed. `cd ui && node ../node_modules/typescript/bin/tsc -b` passed. - The shared query-client and live-update recovery selection passed 96 cases on `49c544cd2`. Token gates passed before the final hook-only changes. - Earlier focused verification passed 57 manifest cases, 69 database/history cases, event-registry cases and route authorization cases, including private-run denial. - Regression coverage includes subscription/history races, task changes during recovery, unfocused run completion, bounded readiness retries, denied reads, expected not-ready responses, changed activity mappings, unchanged panel preservation, old cache scopes, and same-version route or installation changes. - [Current-head CI](https://github.com/paperclipai/paperclip/actions/runs/40942001707) passed: 54 successful checks, two skips, no failures or pending checks. - [Standard and final review](https://github.com/paperclipai/paperclip/pull/18131#issuecomment-6489842168) both scored 5/5 on `5e04d757c`. The final review used the Apex tier, added no comments, and left no unresolved findings. The PR is conflict-free and ready for review. - Local checks stay targeted. GitHub CI provides the broad build, test, typecheck, browser and Runner coverage. No deployment or live-provider acceptance is claimed. <details> <summary>Retained failures and recovery evidence</summary> - `ac9329548` twice failed a browser reload assertion while conversation history was concealed. Master fixed its readiness wait in #17923. The approve journey then passed locally on `fea63a418` with installed Chrome (1 passed, 39 seconds). All eight browser shards passed in later CI. The browser test is outside this diff. - A canary checkout failed with Git HTTP 408; its unchanged-head retry passed. - `de07d09fa` failed the mobile Cases card click after a ten-second stability wait. The same read-only journey passed locally on that exact head (1 passed, 26 seconds). Its unchanged-head CI retry passed 22 cases, with one skip. The original artifact is retained; the cause of that click timeout is not established. - [CI on `49c544cd2`](https://github.com/paperclipai/paperclip/actions/runs/40903679075) passed 54 checks with two skips. CI also passed on `de07d09fa` after the recorded retry. - The `1e59ab441` merge checkout passed all browser and server suites. Runner shard 2 passed 2,252 cases and timed out waiting for a controller-recovery command. The test had landed on master in #18161 after the former branch base. Rebasing aligned the local checkout with that code; Runner shard 2 then passed on `fcf7380d3`. The original timeout cause remains unproven; no unrelated timeout was changed. - [CI on `fcf7380d3`](https://github.com/paperclipai/paperclip/actions/runs/40940235193) recorded runner shutdown signals in typecheck and a workspace shard. A chat recovery test observed a null wake deadline, and one Runner transport fixture failed during close. Those separate assertion failures remain unexplained. All of those jobs passed on the final readiness-key head. Those later passes establish recovery; they do not prove the cause of the earlier assertion failures. - Local browser setup first stopped at pnpm version-download verification, then a sandbox IPC restriction, then a missing bundled browser. Installed pnpm 9.15.4 and Chrome completed the journeys. No dependency files were changed. - Earlier optional-property TypeScript errors were corrected. Embedded PostgreSQL needed its install hook run from its package directory. Older CI also recorded fixture-cleanup lock timeouts, attention-fixture timeouts, and canceled server shards after runner shutdown. The parent fixed its FIFO snapshot-copy failure before merge. The linked verification archive retains earlier combined evidence. </details> ## Risks Activity is a hint and grants no access. Missing plugins, denied reads and unavailable views contribute no panel. Older runs without activity records show no task plugin tab. A used view stays available for the running task; reconnect reconciles readiness. Matching producer and consumer builds are required. No migration, dependency, image publication or deployment is included. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, repository tools and code execution. The exact serving snapshot is not exposed. ## 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>
320 lines
14 KiB
TypeScript
320 lines
14 KiB
TypeScript
import { onlineManager, QueryObserver, MutationObserver, type QueryClient } from "@tanstack/react-query";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { ApiError } from "@/api/client";
|
|
import { ApiUnavailableError } from "@/api/response";
|
|
import { createConnectivityStore, type ConnectivityStore, type ProbeResult } from "./connectivity";
|
|
import {
|
|
bindConnectivity,
|
|
createAppQueryClient,
|
|
reconnectDelayFor,
|
|
retryDelayFor,
|
|
shouldReportQueryError,
|
|
QueryNotReadyError,
|
|
shouldRetryRequest,
|
|
TRANSIENT_RETRY_DELAYS_MS,
|
|
type MutationErrorToast,
|
|
} from "./query-client";
|
|
|
|
const gatewayDown = () => new ApiError("Paperclip is restarting", 503, { error: "tenant_app_unavailable" });
|
|
|
|
describe("retry policy", () => {
|
|
it("retries transient errors for about 30 seconds", () => {
|
|
const error = new ApiUnavailableError(502);
|
|
const retries = TRANSIENT_RETRY_DELAYS_MS.length;
|
|
expect(shouldRetryRequest(retries - 1, error)).toBe(true);
|
|
expect(shouldRetryRequest(retries, error)).toBe(false);
|
|
const total = TRANSIENT_RETRY_DELAYS_MS.reduce((sum, delay) => sum + delay, 0);
|
|
expect(total).toBeGreaterThanOrEqual(25_000);
|
|
expect(total).toBeLessThanOrEqual(35_000);
|
|
});
|
|
|
|
it.each([400, 401, 403, 404, 409, 422])("never retries a %s", (status) => {
|
|
expect(shouldRetryRequest(0, new ApiError("No", status, null))).toBe(false);
|
|
});
|
|
|
|
it("retries the gateway code even when it arrives with a 4xx", () => {
|
|
expect(shouldRetryRequest(0, new ApiError("x", 404, { error: "tenant_app_unavailable" }))).toBe(true);
|
|
});
|
|
|
|
it("retries unexpected failures briefly and aborts never", () => {
|
|
expect(shouldRetryRequest(0, new ApiError("boom", 500, null))).toBe(true);
|
|
expect(shouldRetryRequest(2, new ApiError("boom", 500, null))).toBe(false);
|
|
expect(shouldRetryRequest(0, new DOMException("Aborted", "AbortError"))).toBe(false);
|
|
});
|
|
|
|
it("backs off and honors Retry-After", () => {
|
|
const error = new ApiUnavailableError(503);
|
|
expect(retryDelayFor(0, error)).toBe(1_000);
|
|
expect(retryDelayFor(4, error)).toBe(15_000);
|
|
expect(retryDelayFor(9, error)).toBe(15_000);
|
|
const throttled = new ApiError("Slow down", 429, null, { retryAfterMs: 6_000 });
|
|
expect(retryDelayFor(0, throttled)).toBe(6_000);
|
|
// A rate-limit window longer than the normal backoff is waited out in full.
|
|
const minuteWindow = new ApiError("Slow down", 429, null, { retryAfterMs: 55_000 });
|
|
expect(retryDelayFor(0, minuteWindow)).toBe(55_000);
|
|
expect(retryDelayFor(0, new ApiUnavailableError(503, 20_000))).toBe(20_000);
|
|
expect(reconnectDelayFor(0, throttled)).toBe(6_000);
|
|
expect(reconnectDelayFor(3, new ApiUnavailableError(503))).toBe(10_000);
|
|
});
|
|
|
|
it("reports only unexpected query errors to Sentry", () => {
|
|
expect(shouldReportQueryError(new ApiError("boom", 500, null))).toBe(true);
|
|
expect(shouldReportQueryError(new Error("parse failure"))).toBe(true);
|
|
expect(shouldReportQueryError(new QueryNotReadyError("Not ready"))).toBe(false);
|
|
expect(shouldReportQueryError(gatewayDown())).toBe(false);
|
|
expect(shouldReportQueryError(new TypeError("Failed to fetch"))).toBe(false);
|
|
expect(shouldReportQueryError(new ApiError("Not found", 404, null))).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("createAppQueryClient", () => {
|
|
let store: ConnectivityStore;
|
|
let probe: ReturnType<typeof vi.fn<() => Promise<ProbeResult>>>;
|
|
let client: QueryClient;
|
|
let unbind: () => void;
|
|
let notifyMutationError: ReturnType<typeof vi.fn<(toast: MutationErrorToast) => void>>;
|
|
let reportError: ReturnType<typeof vi.fn<(error: unknown) => void>>;
|
|
|
|
beforeEach(() => {
|
|
vi.useFakeTimers();
|
|
probe = vi.fn<() => Promise<ProbeResult>>().mockResolvedValue({ reachable: false, retryAfterMs: null });
|
|
store = createConnectivityStore({ probe, browserOnline: true });
|
|
notifyMutationError = vi.fn<(toast: MutationErrorToast) => void>();
|
|
reportError = vi.fn<(error: unknown) => void>();
|
|
client = createAppQueryClient({ connectivity: store, notifyMutationError, reportError });
|
|
client.mount();
|
|
unbind = bindConnectivity(client, store);
|
|
});
|
|
|
|
afterEach(() => {
|
|
unbind();
|
|
store.dispose();
|
|
client.unmount();
|
|
client.clear();
|
|
onlineManager.setOnline(true);
|
|
vi.useRealTimers();
|
|
});
|
|
|
|
it("pauses a failing query during an outage and refreshes it on recovery", async () => {
|
|
let down = true;
|
|
const queryFn = vi.fn(async () => {
|
|
if (down) throw gatewayDown();
|
|
return "fresh";
|
|
});
|
|
const observer = new QueryObserver(client, { queryKey: ["issue", "1"], queryFn });
|
|
const unsubscribe = observer.subscribe(() => {});
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
expect(store.getSnapshot().status).toBe("reconnecting");
|
|
expect(onlineManager.isOnline()).toBe(false);
|
|
|
|
await vi.advanceTimersByTimeAsync(30_000);
|
|
// Paused, not failed, and not hammering the server.
|
|
expect(observer.getCurrentResult()).toMatchObject({ status: "pending", fetchStatus: "paused" });
|
|
expect(queryFn).toHaveBeenCalledTimes(1);
|
|
|
|
down = false;
|
|
probe.mockResolvedValue({ reachable: true });
|
|
await vi.advanceTimersByTimeAsync(15_000);
|
|
expect(store.getSnapshot().status).toBe("online");
|
|
expect(observer.getCurrentResult()).toMatchObject({ status: "success", data: "fresh" });
|
|
unsubscribe();
|
|
});
|
|
|
|
it("keeps cached data when a background refetch hits an outage", async () => {
|
|
let down = false;
|
|
const observer = new QueryObserver(client, {
|
|
queryKey: ["issue", "2"],
|
|
queryFn: async () => {
|
|
if (down) throw gatewayDown();
|
|
return "loaded";
|
|
},
|
|
});
|
|
const unsubscribe = observer.subscribe(() => {});
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
down = true;
|
|
void observer.refetch();
|
|
await vi.advanceTimersByTimeAsync(5_000);
|
|
expect(observer.getCurrentResult()).toMatchObject({ data: "loaded", error: null });
|
|
unsubscribe();
|
|
});
|
|
|
|
it("refreshes active queries once on recovery", async () => {
|
|
const queryFn = vi.fn(async () => "ok");
|
|
const observer = new QueryObserver(client, { queryKey: ["list"], queryFn, staleTime: Infinity });
|
|
const unsubscribe = observer.subscribe(() => {});
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
expect(queryFn).toHaveBeenCalledTimes(1);
|
|
store.reportError(gatewayDown());
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
probe.mockResolvedValue({ reachable: true });
|
|
await vi.advanceTimersByTimeAsync(1_000);
|
|
expect(queryFn).toHaveBeenCalledTimes(2);
|
|
unsubscribe();
|
|
});
|
|
|
|
it("does not treat setQueryData as proof the server is back", async () => {
|
|
store.reportError(gatewayDown());
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
client.setQueryData(["optimistic"], "local");
|
|
expect(store.getSnapshot().status).toBe("reconnecting");
|
|
});
|
|
|
|
it("does not infer server recovery from an unrelated in-flight query", async () => {
|
|
let finish!: (value: null) => void;
|
|
const pending = new Promise<null>(resolve => { finish = resolve; });
|
|
const observer = new QueryObserver(client, { queryKey: ["npm-registry"], queryFn: () => pending });
|
|
const unsubscribe = observer.subscribe(() => {});
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
store.reportError(gatewayDown());
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
expect(store.getSnapshot().status).toBe("reconnecting");
|
|
finish(null);
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
expect(store.getSnapshot().status).toBe("reconnecting");
|
|
expect(onlineManager.isOnline()).toBe(false);
|
|
probe.mockResolvedValue({ reachable: true });
|
|
await vi.advanceTimersByTimeAsync(1_000);
|
|
expect(store.getSnapshot().status).toBe("online");
|
|
unsubscribe();
|
|
});
|
|
|
|
it("does not infer server recovery from an unrelated mutation", async () => {
|
|
store.reportError(gatewayDown());
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
const observer = new MutationObserver(client, { mutationFn: async () => "external success" });
|
|
await observer.mutate();
|
|
expect(store.getSnapshot().status).toBe("reconnecting");
|
|
expect(onlineManager.isOnline()).toBe(false);
|
|
});
|
|
|
|
it("fails an ordinary mutation fast during an outage", async () => {
|
|
const mutationFn = vi.fn(async () => {
|
|
throw gatewayDown();
|
|
});
|
|
const observer = new MutationObserver(client, { mutationFn });
|
|
await expect(observer.mutate()).rejects.toBeInstanceOf(ApiError);
|
|
expect(mutationFn).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("pauses an idempotent mutation across an outage and resends it on reconnect", async () => {
|
|
let down = true;
|
|
const mutationFn = vi.fn(async (value: string) => {
|
|
if (down) throw gatewayDown();
|
|
return value;
|
|
});
|
|
const observer = new MutationObserver(client, { mutationFn, meta: { replay: "idempotent" } });
|
|
const result = observer.mutate("title");
|
|
await vi.advanceTimersByTimeAsync(60_000);
|
|
expect(observer.getCurrentResult()).toMatchObject({ status: "pending", isPaused: true });
|
|
expect(mutationFn).toHaveBeenCalledTimes(1);
|
|
expect(store.getSnapshot().pendingWrites).toBe(1);
|
|
|
|
down = false;
|
|
probe.mockResolvedValue({ reachable: true });
|
|
await vi.advanceTimersByTimeAsync(15_000);
|
|
await expect(result).resolves.toBe("title");
|
|
expect(mutationFn).toHaveBeenCalledTimes(2);
|
|
expect(store.getSnapshot().pendingWrites).toBe(0);
|
|
});
|
|
|
|
it("toasts readable copy for a failed opted-in mutation without its own handler", async () => {
|
|
const observer = new MutationObserver(client, {
|
|
mutationFn: async () => {
|
|
throw new ApiError("Title is required", 422, { error: "Title is required" });
|
|
},
|
|
meta: { errorToast: true },
|
|
});
|
|
await observer.mutate().catch(() => undefined);
|
|
expect(notifyMutationError).toHaveBeenCalledWith({ title: "Check your input", body: "Title is required" });
|
|
});
|
|
|
|
it("toasts a transient failure the banner does not cover", async () => {
|
|
const observer = new MutationObserver(client, {
|
|
mutationFn: async () => {
|
|
throw new ApiError("Slow down", 429, null);
|
|
},
|
|
meta: { errorToast: true },
|
|
});
|
|
await observer.mutate().catch(() => undefined);
|
|
expect(notifyMutationError).toHaveBeenCalledWith({
|
|
title: "Connection interrupted",
|
|
body: "Paperclip is temporarily unavailable. Please try again in a moment.",
|
|
});
|
|
});
|
|
|
|
it("does not toast covered transient failures, handled mutations, or mutations that did not opt in", async () => {
|
|
const fail = (error: unknown) => async () => {
|
|
throw error;
|
|
};
|
|
store.reportError(gatewayDown());
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
expect(store.getSnapshot().status).toBe("reconnecting");
|
|
await new MutationObserver(client, {
|
|
mutationFn: fail(gatewayDown()),
|
|
meta: { errorToast: true },
|
|
}).mutate().catch(() => undefined);
|
|
await new MutationObserver(client, {
|
|
mutationFn: fail(new ApiError("No", 403, null)),
|
|
onError: () => undefined,
|
|
meta: { errorToast: true },
|
|
}).mutate().catch(() => undefined);
|
|
// Surfaces that render `mutation.error` inline must not get a second copy.
|
|
await new MutationObserver(client, {
|
|
mutationFn: fail(new ApiError("No", 403, null)),
|
|
}).mutate().catch(() => undefined);
|
|
expect(notifyMutationError).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("reports an unexpected query error to Sentry once per query", async () => {
|
|
const queryFn = vi.fn(async () => {
|
|
throw new ApiError("boom", 500, { error: "boom" });
|
|
});
|
|
const observer = new QueryObserver(client, { queryKey: ["broken"], queryFn, retry: false });
|
|
const unsubscribe = observer.subscribe(() => {});
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
await observer.refetch();
|
|
expect(queryFn).toHaveBeenCalledTimes(2);
|
|
expect(reportError).toHaveBeenCalledTimes(1);
|
|
unsubscribe();
|
|
});
|
|
|
|
it("leaves a cache-only alias stale instead of refetching it without a queryFn", async () => {
|
|
client.setQueryData(["issues", "detail", "issue-uuid"], { id: "issue-uuid", title: "Seeded" });
|
|
const queryFn = vi.fn(async () => "fetched");
|
|
client.setQueryData(["issues", "detail", "PAP-1"], "seeded");
|
|
await client.prefetchQuery({ queryKey: ["issues", "detail", "PAP-1"], queryFn, staleTime: 0 });
|
|
queryFn.mockClear();
|
|
|
|
void client.invalidateQueries({ queryKey: ["issues", "detail"], refetchType: "inactive" });
|
|
void client.refetchQueries({ queryKey: ["issues", "detail"], type: "all" });
|
|
await vi.advanceTimersByTimeAsync(60_000);
|
|
|
|
const alias = client.getQueryCache().find({ queryKey: ["issues", "detail", "issue-uuid"], exact: true });
|
|
expect(alias?.state.status).toBe("success");
|
|
expect(alias?.state.isInvalidated).toBe(true);
|
|
expect(alias?.state.data).toEqual({ id: "issue-uuid", title: "Seeded" });
|
|
expect(queryFn).toHaveBeenCalledTimes(2);
|
|
expect(reportError).not.toHaveBeenCalled();
|
|
|
|
const aliasQueryFn = vi.fn(async () => ({ id: "issue-uuid", title: "Fresh" }));
|
|
const observer = new QueryObserver(client, { queryKey: ["issues", "detail", "issue-uuid"], queryFn: aliasQueryFn });
|
|
const unsubscribe = observer.subscribe(() => {});
|
|
await vi.advanceTimersByTimeAsync(0);
|
|
expect(aliasQueryFn).toHaveBeenCalledTimes(1);
|
|
expect(observer.getCurrentResult().data).toEqual({ id: "issue-uuid", title: "Fresh" });
|
|
unsubscribe();
|
|
});
|
|
|
|
it("keeps the caller's refetch predicate", async () => {
|
|
const keep = vi.fn(async () => "keep");
|
|
const skip = vi.fn(async () => "skip");
|
|
await client.prefetchQuery({ queryKey: ["keep"], queryFn: keep });
|
|
await client.prefetchQuery({ queryKey: ["skip"], queryFn: skip });
|
|
|
|
await client.refetchQueries({ predicate: (query) => query.queryKey[0] === "keep" });
|
|
|
|
expect(keep).toHaveBeenCalledTimes(2);
|
|
expect(skip).toHaveBeenCalledTimes(1);
|
|
});
|
|
});
|