From 276730d63e7198e46665a11686d91c7c5013fe29 Mon Sep 17 00:00:00 2001 From: Constantine Date: Thu, 13 Aug 2026 00:05:53 +0300 Subject: [PATCH] fix(server): recognize cross-package Zod errors (#10168) ## Thinking Path > - Paperclip validates API request bodies with Zod and converts validation failures into client errors. > - The global error handler recognized Zod failures with `instanceof ZodError`. > - Monorepo dependency layouts can provide more than one installed Zod module instance. > - A valid Zod error from another instance fails that identity check and falls through as HTTP 500. > - This pull request keeps the native path and adds a narrow structural fallback for named Zod errors with an issues array. > - The benefit is stable HTTP 400 validation semantics regardless of package-instance identity. ## Linked Issues or Issue Description Related but not duplicate: Refs #6908. That PR catches `instanceof ZodError` inside validation middleware and returns 422; it does not cover errors created by a second Zod module instance, which is the reproduced failure here. **What happened?** An invalid `POST /api/issues/:id/work-products` payload raised a real Zod validation error but returned HTTP 500 because the error came from a different Zod package instance. **Expected behavior** All genuine Zod validation failures return HTTP 400 with validation details, independent of module identity. **Steps to reproduce** 1. Submit a work-product body missing the required `provider`, `externalId`, and `url` fields. 2. Ensure the route schema is resolved from a different installed Zod instance than the server error handler. 3. Observe HTTP 500 before this fix. 4. Observe HTTP 400 after this fix. **Environment** - Paperclip base: `14f20be92b86a49ff2c35495e5b0fa4d719998ef` - Deployment: self-hosted, built from source - Access context: board API - Adapter scope: not adapter-specific - [x] I searched open PRs for `ZodError`, validation errors, and work-product validation and linked related work above. ## What Changed - Add a narrow `readZodIssues` helper that accepts native Zod errors or structurally valid cross-package Zod errors. - Preserve existing HTTP 400 response shape and structured error context. - Add a regression for a Zod error object from another module instance. ## Verification - `pnpm exec vitest run server/src/__tests__/error-handler.test.ts` - `pnpm --filter @paperclipai/server typecheck` - Full upstream CI test/build/e2e matrix passed. - Local post-deploy smoke returned HTTP 400 for the previously failing invalid work-product payload. ## Risks - A deliberately thrown object named `ZodError` with an `issues` array will be treated as a client validation failure. The effect is limited to returning HTTP 400 instead of 500; no authorization or persistence behavior changes. - No schema or migration changes. > This is a bug fix, not roadmap feature work. ## Model Used OpenAI Codex `gpt-5.6-sol`, with tool use, code execution, repository inspection, and independent read-only review agents. ## 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 linked related public work and described the bug in-PR following the bug template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have considered documentation; no user-facing documentation change is required - [x] I have considered and documented 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: cucurigoo --- server/src/__tests__/error-handler.test.ts | 25 ++++++++++++++++++++++ server/src/middleware/error-handler.ts | 12 +++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/error-handler.test.ts b/server/src/__tests__/error-handler.test.ts index 3d58f4fa27..cfd65a23a2 100644 --- a/server/src/__tests__/error-handler.test.ts +++ b/server/src/__tests__/error-handler.test.ts @@ -87,6 +87,31 @@ describe("errorHandler", () => { expect(res.__errorContext?.error?.message).toBe("db exploded"); }); + it("returns 400 for Zod validation errors from another module instance", () => { + const req = makeReq(); + const res = makeRes() as any; + const next = vi.fn() as unknown as NextFunction; + const issue = { + code: "invalid_type", + expected: "string", + received: "undefined", + path: ["provider"], + message: "Required", + }; + const err = Object.assign(new Error("Validation failed"), { + name: "ZodError", + issues: [issue], + errors: [issue], + }); + + errorHandler(err, req, res, next); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ error: "Validation error", details: [issue] }); + expect(res.err).toBeUndefined(); + expect(res.__errorContext).toBeUndefined(); + }); + it("records responsible-user denial codes on the active agent run", () => { const db = { marker: "db" }; const req = { diff --git a/server/src/middleware/error-handler.ts b/server/src/middleware/error-handler.ts index 8937dfdcbc..5ba9ad5821 100644 --- a/server/src/middleware/error-handler.ts +++ b/server/src/middleware/error-handler.ts @@ -23,6 +23,13 @@ function isRedactedSkillPolicyDenial(details: Record | null) { return details?.code === "skill_policy_denied"; } +function readZodIssues(err: unknown): unknown[] | null { + if (err instanceof ZodError) return err.issues; + if (!err || typeof err !== "object" || (err as { name?: unknown }).name !== "ZodError") return null; + const issues = (err as { issues?: unknown }).issues; + return Array.isArray(issues) ? issues : null; +} + function attachErrorContext( req: Request, res: Response, @@ -117,8 +124,9 @@ export function errorHandler( return; } - if (err instanceof ZodError) { - res.status(400).json({ error: "Validation error", details: err.errors }); + const zodIssues = readZodIssues(err); + if (zodIssues) { + res.status(400).json({ error: "Validation error", details: zodIssues }); return; }