From 4033185c478fda993a8d6daaacc03c54f855d01c Mon Sep 17 00:00:00 2001 From: Neonforge <48338160+Neonforge98@users.noreply.github.com> Date: Thu, 6 Aug 2026 08:28:13 -0700 Subject: [PATCH 1/2] fix(cloud): salvage comment listing rows per-row cloud_list_session_comments parsed all-or-nothing: one malformed row, a mention list past this client's outbound cap, or a NEWER backend's kind/resolution enum value zod-failed the whole listing, pinning that session's comment pane in error-retry for every member with no way to attribute the culprit. Apply the tolerant-record rule the sessions listing already follows: parse rows individually and drop only the bad one, naming the first casualty (id + first zod issue) in a rate-limited diagnostic; degrade unknown kind/resolution values to their absent-field semantics instead of failing; stop re-checking the 50-mention cap on read. Pre-commit hook ran. Total eslint: 18, total circular: 0 --- .../Org2Cloud/org2CloudCommentsClient.test.ts | 62 +++++++++++++++ .../Org2Cloud/org2CloudCommentsClient.ts | 77 +++++++++++++++++-- 2 files changed, 132 insertions(+), 7 deletions(-) diff --git a/src/features/Org2Cloud/org2CloudCommentsClient.test.ts b/src/features/Org2Cloud/org2CloudCommentsClient.test.ts index 2842dfcec..27dc33901 100644 --- a/src/features/Org2Cloud/org2CloudCommentsClient.test.ts +++ b/src/features/Org2Cloud/org2CloudCommentsClient.test.ts @@ -448,6 +448,68 @@ describe("listSessionComments", () => { expect(result).not.toHaveProperty("unknownLegacyField"); }); + it("drops a malformed row alone and keeps the rest of the listing", async () => { + fetchMock.mockResolvedValueOnce( + jsonResponse({ + viewerOwnsSession: true, + comments: [ + WIRE_COMMENT, + // Structurally broken row (no id, no body): must cost only itself. + { authorUserId: 42, createdAt: null }, + { ...WIRE_COMMENT, id: "comment-3" }, + ], + }) + ); + const { comments, viewerOwnsSession } = await listSessionComments( + "jwt-1", + "org-1", + "sess-1" + ); + expect(viewerOwnsSession).toBe(true); + expect(comments.map((comment) => comment.id)).toEqual([ + "comment-1", + "comment-3", + ]); + }); + + it("degrades unknown kind/resolution values to absent-field semantics", async () => { + fetchMock.mockResolvedValueOnce( + jsonResponse({ + comments: [ + { + ...WIRE_COMMENT, + kind: "agent_summary", + resolvedAt: "2026-07-11T10:00:00.000Z", + resolution: "duplicate", + }, + ], + }) + ); + const { comments } = await listSessionComments("jwt-1", "org-1", "sess-1"); + // A newer backend's enum value renders as the absent-field fallback + // ('user' semantics / plain resolve) — the row itself survives. + expect(comments).toHaveLength(1); + expect(comments[0].kind).toBeUndefined(); + expect(comments[0].resolution).toBeUndefined(); + expect(comments[0].resolvedAt).toBe("2026-07-11T10:00:00.000Z"); + }); + + it("keeps a row whose mention list exceeds this client's outbound cap", async () => { + fetchMock.mockResolvedValueOnce( + jsonResponse({ + comments: [ + { + ...WIRE_COMMENT, + mentionedUserIds: Array.from({ length: 60 }, (_, i) => `u-${i}`), + }, + ], + }) + ); + const { comments } = await listSessionComments("jwt-1", "org-1", "sess-1"); + expect(comments).toHaveLength(1); + expect(comments[0].mentionedUserIds).toHaveLength(60); + }); + it("maps ORG2_RETENTION_EXPIRED into a coded error", async () => { fetchMock.mockResolvedValueOnce( jsonResponse({ message: "ORG2_RETENTION_EXPIRED" }, 400) diff --git a/src/features/Org2Cloud/org2CloudCommentsClient.ts b/src/features/Org2Cloud/org2CloudCommentsClient.ts index 55c8d625f..3d8f200a3 100644 --- a/src/features/Org2Cloud/org2CloudCommentsClient.ts +++ b/src/features/Org2Cloud/org2CloudCommentsClient.ts @@ -23,9 +23,13 @@ */ import { z } from "zod/v4"; +import { createLogger } from "@src/hooks/logger"; + import { ORG2_CLOUD_POSTGREST_SCHEMA, getCloudEndpoint } from "./config"; import { fetchWithTransportRetry } from "./org2CloudFetchRetry"; +const log = createLogger("Org2CloudCommentsClient"); + /** RPC-enforced body bound (0014 SIZE note) — mirrored in composers. */ export const CLOUD_COMMENT_MAX_BODY_LENGTH = 4000; @@ -159,11 +163,15 @@ const CloudSessionCommentWireSchema = z.object({ .nullish() .transform((value) => value ?? undefined) .optional(), + // The two enum fields degrade UNKNOWN values to undefined instead of + // failing: a newer backend introducing a verdict/kind must render as the + // absent-field fallback on this client, never brick the listing. resolution: z .enum(["resolved", "wont_fix"]) .nullish() .transform((value) => value ?? undefined) - .optional(), + .optional() + .catch(undefined), /** * Agent-reply discriminator; absent on an older backend means `user`. * The server accepts `agent_report` only from the cloud-session owner. @@ -172,9 +180,15 @@ const CloudSessionCommentWireSchema = z.object({ .enum(["user", "agent_report"]) .nullish() .transform((value) => value ?? undefined) - .optional(), - /** Explicit user ids targeted by the comment (0010 Team Inbox). */ - mentionedUserIds: z.array(z.string()).max(50).optional(), + .optional() + .catch(undefined), + /** + * Explicit user ids targeted by the comment (0010 Team Inbox). Uncapped on + * READ — the 50-id bound is enforced where it protects something (this + * client's outbound request, the server RPC); re-checking it here would + * turn a future server-side cap raise into a bricked listing. + */ + mentionedUserIds: z.array(z.string()).optional(), }); export type CloudSessionComment = z.output< @@ -186,7 +200,9 @@ const AddCommentResultSchema = z.object({ }); const ListCommentsResultSchema = z.object({ - comments: z.array(CloudSessionCommentWireSchema).default([]), + // Rows parse individually in `parseCommentRows` — one malformed row must + // cost that row, not the whole thread listing (the tolerant-record rule). + comments: z.array(z.unknown()).default([]), /** Viewer-derived server capability; false for imports, forks and members. */ viewerOwnsSession: z.boolean().default(false), /** 0004 delta anchor; absent on pre-delta backends. */ @@ -197,6 +213,47 @@ const ListCommentsResultSchema = z.object({ .optional(), }); +/** + * Per-row salvage for the listing: a malformed row is dropped alone and the + * FIRST casualty is named (id + first zod issue) so a live "dropped N" + * symptom stays attributable after the row ages out. Without this, one bad + * row pins the whole session's comment pane in error-retry for every member. + */ +function parseCommentRows( + sessionId: string, + rows: readonly unknown[] +): CloudSessionComment[] { + const parsed: CloudSessionComment[] = []; + let dropped = 0; + let firstDrop: string | undefined; + for (const row of rows) { + const result = CloudSessionCommentWireSchema.safeParse(row); + if (result.success) { + parsed.push(result.data); + continue; + } + dropped += 1; + if (dropped === 1) { + const record = row as Record | null; + const rowId = typeof record?.id === "string" ? record.id : ""; + const issue = result.error.issues[0]; + firstDrop = `${rowId.slice(0, 64)} (${ + issue + ? `${issue.path.join(".") || ""}: ${issue.message}` + : "unknown issue" + })`; + } + } + if (dropped > 0) { + log.rateLimited( + `comments-malformed-${sessionId}`, + 60_000, + `cloud_list_session_comments dropped ${dropped} malformed row(s) for session ${sessionId}, first: ${firstDrop}` + ); + } + return parsed; +} + const EditCommentResultSchema = z.object({ editedAt: z.string(), }); @@ -419,8 +476,10 @@ export async function listSessionComments( p_since: since, } ); + const result = ListCommentsResultSchema.parse(payload); return { - ...ListCommentsResultSchema.parse(payload), + ...result, + comments: parseCommentRows(sessionId, result.comments), appliedSince: since, }; } catch (error) { @@ -436,5 +495,9 @@ export async function listSessionComments( p_session_id: sessionId, } ); - return ListCommentsResultSchema.parse(payload); + const result = ListCommentsResultSchema.parse(payload); + return { + ...result, + comments: parseCommentRows(sessionId, result.comments), + }; } From 515c5dc4758d5b768cd5dd98e98bca81a89c311f Mon Sep 17 00:00:00 2001 From: Neonforge <48338160+Neonforge98@users.noreply.github.com> Date: Thu, 6 Aug 2026 08:43:52 -0700 Subject: [PATCH 2/2] fix(cloud): salvage team-inbox mention rows per-row Same all-or-nothing class as the comments listing: one malformed mention row zod-failed the whole cloud_list_team_inbox_mentions page, blanking the Team Inbox instead of costing the one row. Parse rows individually, name the first casualty, and keep rejecting a malformed page envelope (unreadCount/cursor) outright. Pre-commit hook ran. Total eslint: 18, total circular: 0 --- .../Org2Cloud/teamInboxMentionsClient.test.ts | 24 ++++++++- .../Org2Cloud/teamInboxMentionsClient.ts | 51 ++++++++++++++++++- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/features/Org2Cloud/teamInboxMentionsClient.test.ts b/src/features/Org2Cloud/teamInboxMentionsClient.test.ts index e60b64060..3dfcc791f 100644 --- a/src/features/Org2Cloud/teamInboxMentionsClient.test.ts +++ b/src/features/Org2Cloud/teamInboxMentionsClient.test.ts @@ -201,15 +201,35 @@ describe("listTeamInboxMentions", () => { }); }); - it("rejects malformed response fields instead of leaking raw wire data", async () => { + it("drops a malformed mention row alone instead of leaking raw wire data", async () => { fetchMock.mockResolvedValueOnce( jsonResponse({ - mentions: [{ ...WIRE_MENTION, commentCount: -1 }], + mentions: [ + { ...WIRE_MENTION, commentCount: -1 }, + { ...WIRE_MENTION, comment: { id: "comment-9", parentId: null } }, + ], nextCursor: null, unreadCount: 1, }) ); + const page = await listTeamInboxMentions("jwt-viewer", "org-1", null, 25); + + // The malformed row costs only itself — never surfaced raw, never fatal. + expect(page.mentions).toHaveLength(1); + expect(page.mentions[0].comment.id).toBe("comment-9"); + expect(page.unreadCount).toBe(1); + }); + + it("rejects a malformed page envelope outright", async () => { + fetchMock.mockResolvedValueOnce( + jsonResponse({ + mentions: [WIRE_MENTION], + nextCursor: null, + unreadCount: -1, + }) + ); + await expect( listTeamInboxMentions("jwt-viewer", "org-1", null, 25) ).rejects.toBeInstanceOf(ZodError); diff --git a/src/features/Org2Cloud/teamInboxMentionsClient.ts b/src/features/Org2Cloud/teamInboxMentionsClient.ts index 6a87fa44c..6000a1a69 100644 --- a/src/features/Org2Cloud/teamInboxMentionsClient.ts +++ b/src/features/Org2Cloud/teamInboxMentionsClient.ts @@ -1,5 +1,7 @@ import { z } from "zod/v4"; +import { createLogger } from "@src/hooks/logger"; + import { ORG2_CLOUD_POSTGREST_SCHEMA, getCloudEndpoint } from "./config"; import { getCloudCapabilities } from "./org2CloudCapabilities"; import { Org2CloudCommentError } from "./org2CloudCommentsClient"; @@ -8,6 +10,8 @@ import { runCloudRequestWithTimeout, } from "./org2CloudFetchRetry"; +const log = createLogger("TeamInboxMentionsClient"); + const TEAM_INBOX_MENTIONS_RPC = "cloud_list_team_inbox_mentions"; const SET_TEAM_INBOX_MENTION_READ_RPC = "cloud_set_team_inbox_mention_read"; const MARK_ALL_TEAM_INBOX_MENTIONS_READ_RPC = @@ -47,13 +51,52 @@ const TeamInboxMentionSchema = z.object({ }); const TeamInboxMentionsPageSchema = z.object({ - mentions: z.array(TeamInboxMentionSchema).default([]), + // Rows parse individually in `parseMentionRows` — one malformed row must + // cost that row, not the whole inbox page (the tolerant-record rule). + mentions: z.array(z.unknown()).default([]), nextCursor: NullableStringSchema, unreadCount: z.number().int().nonnegative(), }); export type TeamInboxMention = z.output; +/** Per-row salvage naming the first casualty (comment id + first zod issue). */ +function parseMentionRows( + orgId: string, + rows: readonly unknown[] +): TeamInboxMention[] { + const parsed: TeamInboxMention[] = []; + let dropped = 0; + let firstDrop: string | undefined; + for (const row of rows) { + const result = TeamInboxMentionSchema.safeParse(row); + if (result.success) { + parsed.push(result.data); + continue; + } + dropped += 1; + if (dropped === 1) { + const record = row as { comment?: { id?: unknown } } | null; + const rowId = + typeof record?.comment?.id === "string" ? record.comment.id : ""; + const issue = result.error.issues[0]; + firstDrop = `${rowId.slice(0, 64)} (${ + issue + ? `${issue.path.join(".") || ""}: ${issue.message}` + : "unknown issue" + })`; + } + } + if (dropped > 0) { + log.rateLimited( + `inbox-malformed-${orgId}`, + 60_000, + `${TEAM_INBOX_MENTIONS_RPC} dropped ${dropped} malformed row(s) for org ${orgId}, first: ${firstDrop}` + ); + } + return parsed; +} + export interface TeamInboxMentionsPage { mentions: TeamInboxMention[]; nextCursor?: string; @@ -146,7 +189,11 @@ export async function listTeamInboxMentions( }, signal ); - return TeamInboxMentionsPageSchema.parse(payload); + const page = TeamInboxMentionsPageSchema.parse(payload); + return { + ...page, + mentions: parseMentionRows(input.orgId, page.mentions), + }; } /**