Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions src/features/Org2Cloud/org2CloudCommentsClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
77 changes: 70 additions & 7 deletions src/features/Org2Cloud/org2CloudCommentsClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand All @@ -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<
Expand All @@ -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. */
Expand All @@ -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<string, unknown> | null;
const rowId = typeof record?.id === "string" ? record.id : "<no id>";
const issue = result.error.issues[0];
firstDrop = `${rowId.slice(0, 64)} (${
issue
? `${issue.path.join(".") || "<root>"}: ${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(),
});
Expand Down Expand Up @@ -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) {
Expand All @@ -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),
};
}
24 changes: 22 additions & 2 deletions src/features/Org2Cloud/teamInboxMentionsClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
51 changes: 49 additions & 2 deletions src/features/Org2Cloud/teamInboxMentionsClient.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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 =
Expand Down Expand Up @@ -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<typeof TeamInboxMentionSchema>;

/** 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 : "<no id>";
const issue = result.error.issues[0];
firstDrop = `${rowId.slice(0, 64)} (${
issue
? `${issue.path.join(".") || "<root>"}: ${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;
Expand Down Expand Up @@ -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),
};
}

/**
Expand Down
Loading