mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-01 15:35:12 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Contributors need fast, reliable CI feedback. > - Diff-aware CI selects test files and partitions them into runner jobs. > - Stale cost estimates put several slow suites on the same runner. Broad imports also couple small tests to large modules. > - This PR refreshes test costs, runs independent graph discovery in parallel, and narrows three dependency boundaries. > - Selection safeguards and CODEOWNER-only AWS routing stay in force. ## Linked Issues or Issue Description Refs #18398. This follows the audit of [PR #18427's verification run](https://github.com/paperclipai/paperclip/actions/runs/41169978784). The audit found 403 selected general-server files without timing weights. Three suites measured at 232s, 122s and 116s shared one shard because each received a roughly 2s fallback weight. Local graph discovery across four projects took 43.7s serially and 21.7s with two workers. A reverse-order repeat took 37.7s serially and 22.6s with two workers. Both comparisons produced identical selection results. This is a local measurement, not a claim about production CI speed. ## What Changed - Refresh general-server costs for 1,285 of 1,309 current files and serialized costs for all 163 files from two successful protected-master Cloud readiness runs. Retain prior costs for unmeasured files, including dedicated chat and native-runner suites. - Record passing server file costs, including collection, setup and environment preparation. A weekly hosted workflow validates trusted source runs and opens a timing-only PR. It tests shard coverage, pauses while a refresh PR is open, and never merges automatically. - Discover at most two project graphs concurrently in isolated, heap-limited Node processes. Cache repeated inventory reads within one selection. Worker errors and timeouts retain the full task plan. - Fetch only the merge commit and parents for the selector. A real shallow-clone regression proves the complete diff still includes more than 300 files, renames and deletions. - Share the identical log viewer and run details between both AgentDetail pages. The viewer test imports the shared implementation and runs each of its 13 distinct cases once. - Extract the pure API error class while preserving the API client's public exports. Import delivery work signals through their existing narrow DB export. ## Verification - PASS: `pnpm -r typecheck` and `pnpm build`. - PASS: 284 CI selection, timing, shallow-diff, sharding and routing tests. - PASS: 74 further cache, routing and graph-proof tests. - PASS: 94 targeted UI assertions across the viewer, run invocation, API errors/client and progress helpers. All 76 assertions in the 12 server suites made independently selectable also pass. - PASS: `pnpm check:token-gates`, module-boundary checks, YAML parsing and `git diff --check`. - PASS: [Full PR CI and required `ci / verify` aggregate](https://github.com/paperclipai/paperclip/actions/runs/41206683345), including all 12 general-server shards, all nine serialized shards, workspace groups, chat suites, Runner lanes and build checks. - PASS: [GitHub hosted real-workspace selector and cache/routing qualification](https://github.com/paperclipai/paperclip/actions/runs/41206683205). - PASS: Both Greptile and Paperclip reviews are 5/5 with no actionable findings or inline comments on `7dc65c51b8`. The PR has no merge conflicts. - PASS: The production refresh parser accepted all 107 structured file-cost records from a real successful server CI job, including setup/loading cost. - Local `pnpm test:run` completed all six chat shards successfully. The duplicate monolithic local run was stopped after the complete remote suite passed; it was not completed locally. The focused log-viewer command improved from 18.84s (26 duplicate variant cases) to 8.49s (all 13 distinct behavior cases). The dependency audit confirms 12 server files and the API-error test no longer need conservative always-run retention. Repartitioning the audited PR with the new costs changes the slowest shard from 810s to 591s of summed historical test execution (1,124 of 1,142 files measured). This is a workload simulation, not measured new CI wall time. ## Risks - Two graph processes use more peak memory. Each has a 2 GiB heap limit and a three-minute timeout. Any failure preserves the original task plan. Real workspace qualification runs in GitHub CI. - Historical logs contain test/hook costs only. New reporter records add loading/setup costs after merge. The manifests identify measurement coverage and source runs. These weights change placement, never permission to skip tests. - Runner load adds timing noise. The refresh uses medians and requires at least 90% measurement coverage in each lane. - GitHub may require approval to run checks on a PR created with `GITHUB_TOKEN`. Refreshes remain reviewable and do not merge themselves. - The shared viewer is a mechanical extraction. Both pages retain their existing exports, and the behavior assertions remain covered. AWS runner authorization is unchanged. ## Model Used OpenAI Codex, GPT-6, with repository inspection, reasoning, code execution and GitHub tools. The exact deployment ID and context-window size 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 references) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (focused checks and six chat shards; complete suite passed in CI as detailed above) - [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>
192 lines
8.0 KiB
TypeScript
192 lines
8.0 KiB
TypeScript
import { describe, expect, it } from "vitest";
|
||
import { ApiError } from "./api-error";
|
||
import {
|
||
apiErrorMessage,
|
||
classifyError,
|
||
describeError,
|
||
errorCodes,
|
||
isConnectivityError,
|
||
isRawErrorCode,
|
||
parseRetryAfter,
|
||
type ErrorKind,
|
||
} from "./errors";
|
||
import { ApiUnavailableError } from "./response";
|
||
|
||
function apiError(status: number, body: unknown = null) {
|
||
return new ApiError(apiErrorMessage(status, body), status, body);
|
||
}
|
||
|
||
describe("classifyError", () => {
|
||
it.each<[string, unknown, ErrorKind]>([
|
||
["a browser network failure", new TypeError("Failed to fetch"), "transient"],
|
||
["a Safari network failure", new TypeError("Load failed"), "transient"],
|
||
["a proxy HTML page", new ApiUnavailableError(502), "transient"],
|
||
["408", apiError(408), "transient"],
|
||
["425", apiError(425), "transient"],
|
||
["429", apiError(429, { error: "Too many requests" }), "transient"],
|
||
["502", apiError(502), "transient"],
|
||
["503", apiError(503, { error: "Service unavailable" }), "transient"],
|
||
["504", apiError(504), "transient"],
|
||
["401", apiError(401, { error: "Unauthorized" }), "auth"],
|
||
["403", apiError(403, { error: "Forbidden" }), "forbidden"],
|
||
["404", apiError(404, { error: "Issue not found" }), "not_found"],
|
||
["410", apiError(410), "not_found"],
|
||
["409", apiError(409, { error: "Document is locked" }), "conflict"],
|
||
["412", apiError(412), "conflict"],
|
||
["400", apiError(400, { error: "Title is required" }), "invalid"],
|
||
["422", apiError(422), "invalid"],
|
||
["500", apiError(500, { error: "boom" }), "unknown"],
|
||
["an abort", new DOMException("Aborted", "AbortError"), "aborted"],
|
||
["a programming TypeError", new TypeError("Cannot read properties of undefined"), "unknown"],
|
||
["a plain Error", new Error("nope"), "unknown"],
|
||
["undefined", undefined, "unknown"],
|
||
])("classifies %s", (_label, error, kind) => {
|
||
expect(classifyError(error)).toBe(kind);
|
||
});
|
||
|
||
it.each([200, 400, 403, 404, 409, 500])(
|
||
"treats the gateway's tenant_app_unavailable as transient in any status (%s)",
|
||
(status) => {
|
||
expect(classifyError(apiError(status, { error: "tenant_app_unavailable" }))).toBe("transient");
|
||
expect(classifyError(apiError(status, { error: "tenant_app_starting" }))).toBe("transient");
|
||
},
|
||
);
|
||
|
||
it.each(["WORKER_UNAVAILABLE", "TIMEOUT"])("treats plugin bridge %s as transient", (code) => {
|
||
expect(classifyError(apiError(500, { code, message: "Plugin worker is restarting" }))).toBe("transient");
|
||
});
|
||
|
||
it("reads a code from details, as HttpErrors serialize it", () => {
|
||
const error = apiError(409, { error: "Locked", details: { code: "tenant_app_starting" } });
|
||
expect(classifyError(error)).toBe("transient");
|
||
});
|
||
|
||
it("classifies structurally, without the ApiError class", () => {
|
||
expect(classifyError({ status: 503, body: null })).toBe("transient");
|
||
expect(classifyError({ name: "AuthApiError", status: 401, code: null })).toBe("auth");
|
||
});
|
||
});
|
||
|
||
describe("isConnectivityError", () => {
|
||
it("is true for server-wide outages only", () => {
|
||
expect(isConnectivityError(new TypeError("Failed to fetch"))).toBe(true);
|
||
expect(isConnectivityError(new ApiUnavailableError(200))).toBe(true);
|
||
expect(isConnectivityError(apiError(503))).toBe(true);
|
||
expect(isConnectivityError(apiError(404, { error: "tenant_app_unavailable" }))).toBe(true);
|
||
// Transient, but one route: left to query retry, not the app banner.
|
||
expect(isConnectivityError(apiError(429))).toBe(false);
|
||
expect(isConnectivityError(apiError(503, { code: "WORKER_UNAVAILABLE", message: "x" }))).toBe(false);
|
||
expect(isConnectivityError(apiError(500))).toBe(false);
|
||
expect(isConnectivityError(apiError(403))).toBe(false);
|
||
});
|
||
});
|
||
|
||
describe("describeError", () => {
|
||
const rawCode = /\b[a-z]+(?:_[a-z]+)+\b|\b[A-Z]+_[A-Z]+\b/;
|
||
|
||
it("never returns a raw code", () => {
|
||
const errors = [
|
||
apiError(503, { error: "tenant_app_unavailable" }),
|
||
apiError(200, { error: "tenant_app_starting" }),
|
||
apiError(404, { error: "issue_not_found" }),
|
||
apiError(500, { error: "internal_error" }),
|
||
apiError(503, { code: "WORKER_UNAVAILABLE", message: "WORKER_UNAVAILABLE" }),
|
||
new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" }),
|
||
];
|
||
for (const error of errors) {
|
||
const { title, body } = describeError(error);
|
||
expect(title).not.toMatch(rawCode);
|
||
expect(body).not.toMatch(rawCode);
|
||
}
|
||
});
|
||
|
||
it("uses the shared transient copy for outages", () => {
|
||
expect(describeError(new ApiUnavailableError(502))).toEqual({
|
||
title: "Connection interrupted",
|
||
body: "Paperclip is temporarily unavailable. Please try again in a moment.",
|
||
retryable: true,
|
||
});
|
||
expect(describeError(apiError(503, { error: "tenant_app_unavailable" }))).toMatchObject({
|
||
title: "Reconnecting to Paperclip",
|
||
retryable: true,
|
||
});
|
||
});
|
||
|
||
it("keeps a readable server message for client errors", () => {
|
||
expect(describeError(apiError(400, { error: "Title is required" }))).toEqual({
|
||
title: "Check your input",
|
||
body: "Title is required",
|
||
retryable: false,
|
||
});
|
||
});
|
||
|
||
it("falls back to the kind's copy when the server sent a code or nothing", () => {
|
||
expect(describeError(apiError(404, { error: "issue_not_found" })).body).toBe("This item doesn’t exist or was moved.");
|
||
expect(describeError(apiError(403, null))).toMatchObject({ title: "Not allowed", retryable: false });
|
||
expect(describeError(new Error("Something odd"))).toMatchObject({ body: "Something odd", retryable: true });
|
||
expect(describeError(undefined)).toMatchObject({ title: "Something went wrong", retryable: true });
|
||
});
|
||
|
||
it("names the action in the title for real failures, not outages", () => {
|
||
expect(describeError(apiError(409, { error: "Document is locked" }), { action: "save the document" })).toEqual({
|
||
title: "Couldn't save the document",
|
||
body: "Document is locked",
|
||
retryable: false,
|
||
});
|
||
expect(describeError(apiError(503), { action: "save the document" }).title).toBe("Connection interrupted");
|
||
});
|
||
});
|
||
|
||
describe("ApiError", () => {
|
||
it("keeps the raw body.error as code and a readable message", () => {
|
||
const error = apiError(503, { error: "tenant_app_unavailable" });
|
||
expect(error.code).toBe("tenant_app_unavailable");
|
||
expect(error.message).toBe("Paperclip is restarting or updating. Please try again in a moment.");
|
||
});
|
||
|
||
it("passes a readable server message through unchanged", () => {
|
||
const error = apiError(409, { error: "Document is locked" });
|
||
expect(error.code).toBe("Document is locked");
|
||
expect(error.message).toBe("Document is locked");
|
||
});
|
||
|
||
it("replaces the generic status line with readable copy", () => {
|
||
expect(apiError(404).message).toBe("This item doesn’t exist or was moved.");
|
||
expect(apiError(404).code).toBeNull();
|
||
});
|
||
|
||
it("exposes every code for matching", () => {
|
||
expect(errorCodes(apiError(403, { error: "Denied", code: "interaction_human_only", details: { code: "x_y" } })))
|
||
.toEqual(["Denied", "interaction_human_only", "x_y"]);
|
||
});
|
||
});
|
||
|
||
describe("isRawErrorCode", () => {
|
||
it.each(["tenant_app_unavailable", "WORKER_UNAVAILABLE", "TIMEOUT", "issue.not_found"])("%s is a code", (value) => {
|
||
expect(isRawErrorCode(value)).toBe(true);
|
||
});
|
||
|
||
it.each(["Forbidden", "Unauthorized", "Document is locked", "Request failed: 503", ""])("%s is not a code", (value) => {
|
||
expect(isRawErrorCode(value)).toBe(false);
|
||
});
|
||
});
|
||
|
||
describe("parseRetryAfter", () => {
|
||
it("parses delta-seconds", () => {
|
||
expect(parseRetryAfter("5")).toBe(5_000);
|
||
expect(parseRetryAfter(" 1.5 ")).toBe(1_500);
|
||
});
|
||
|
||
it("parses an HTTP date relative to now", () => {
|
||
const now = Date.parse("2026-10-09T12:00:00Z");
|
||
expect(parseRetryAfter("Fri, 09 Oct 2026 12:00:30 GMT", now)).toBe(30_000);
|
||
expect(parseRetryAfter("Fri, 09 Oct 2026 11:00:00 GMT", now)).toBe(0);
|
||
});
|
||
|
||
it("caps absurd values and ignores garbage", () => {
|
||
expect(parseRetryAfter("86400")).toBe(5 * 60_000);
|
||
expect(parseRetryAfter("soon")).toBeNull();
|
||
expect(parseRetryAfter(null)).toBeNull();
|
||
});
|
||
});
|