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
18 changes: 16 additions & 2 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1401,19 +1401,33 @@ export async function listNotificationDeliveriesForRecipient(
return rows.map(toNotificationDeliveryRecord);
}

export const MAX_NOTIFICATION_MARK_READ_IDS = 100;
export const MAX_NOTIFICATION_DELIVERY_ID_LENGTH = 128;

// Marks a recipient's delivered notifications read (the badge-clear action). Scoped to recipientLogin so a
// caller can never clear another user's notifications. Returns the number of rows transitioned.
// caller can never clear another user's notifications. Passing an empty ids array is a no-op.
// Returns the number of rows transitioned.
export async function markNotificationDeliveriesRead(
env: Env,
recipientLogin: string,
ids?: string[],
): Promise<number> {
if (ids) {
if (ids.length === 0) return 0;
if (ids.length > MAX_NOTIFICATION_MARK_READ_IDS) {
throw new RangeError(`ids must contain at most ${MAX_NOTIFICATION_MARK_READ_IDS} entries`);
}
if (ids.some((id) => id.length > MAX_NOTIFICATION_DELIVERY_ID_LENGTH)) {
throw new RangeError(`ids entries must be at most ${MAX_NOTIFICATION_DELIVERY_ID_LENGTH} characters`);
}
}

const db = getDb(env.DB);
const conditions: SQL[] = [
eq(notificationDeliveries.recipientLogin, recipientLogin.toLowerCase()),
eq(notificationDeliveries.status, "delivered"),
];
if (ids && ids.length > 0) conditions.push(inArray(notificationDeliveries.id, ids));
if (ids) conditions.push(inArray(notificationDeliveries.id, ids));
const updated = await db
.update(notificationDeliveries)
.set({ status: "read", readAt: nowIso() })
Expand Down
7 changes: 6 additions & 1 deletion src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import {
listRepoSyncSegments,
listRepoSyncStates,
listRepositories,
MAX_NOTIFICATION_DELIVERY_ID_LENGTH,
MAX_NOTIFICATION_MARK_READ_IDS,
markNotificationDeliveriesRead,
recordProductUsageEvent,
} from "../db/repositories";
Expand Down Expand Up @@ -426,7 +428,10 @@ const listNotificationsShape = {

const markNotificationsReadShape = {
login: z.string().min(1),
ids: z.array(z.string().min(1)).optional(),
ids: z
.array(z.string().min(1).max(MAX_NOTIFICATION_DELIVERY_ID_LENGTH))
.max(MAX_NOTIFICATION_MARK_READ_IDS)
.optional(),
};

const explainRepoDecisionOutputSchema = {
Expand Down
27 changes: 26 additions & 1 deletion test/unit/mcp-notifications.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
import { describe, expect, it } from "vitest";
import { GittensoryMcp } from "../../src/mcp/server";
import { createSessionForGitHubUser, type AuthIdentity } from "../../src/auth/security";
import { insertNotificationDeliveryIfAbsent, markNotificationDeliveryDelivered } from "../../src/db/repositories";
import {
MAX_NOTIFICATION_DELIVERY_ID_LENGTH,
MAX_NOTIFICATION_MARK_READ_IDS,
insertNotificationDeliveryIfAbsent,
markNotificationDeliveryDelivered,
} from "../../src/db/repositories";
import { createTestEnv } from "../helpers/d1";

async function connect(env: Env, identity?: AuthIdentity) {
Expand Down Expand Up @@ -50,6 +55,26 @@ describe("MCP notification tools", () => {
expect((after.structuredContent as { unreadCount: number }).unreadCount).toBe(0);
});

it("rejects oversized mark-read id filters", async () => {
const env = createTestEnv();
const client = await connect(env);

const tooManyIds = await client.callTool({
name: "gittensory_mark_notifications_read",
arguments: {
login: "miner",
ids: Array.from({ length: MAX_NOTIFICATION_MARK_READ_IDS + 1 }, (_, index) => `id-${index}`),
},
});
expect(tooManyIds.isError).toBe(true);

const tooLongId = await client.callTool({
name: "gittensory_mark_notifications_read",
arguments: { login: "miner", ids: ["x".repeat(MAX_NOTIFICATION_DELIVERY_ID_LENGTH + 1)] },
});
expect(tooLongId.isError).toBe(true);
});

it("returns a contributor's own post-merge outcomes via gittensory_pr_outcome (#702)", async () => {
const env = createTestEnv();
// Seed a merged-PR outcome + a changes-requested delivery; only the merge should surface as an outcome.
Expand Down
21 changes: 21 additions & 0 deletions test/unit/notifications-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
} from "../../src/notifications/service";
import {
getNotificationDeliveryById,
MAX_NOTIFICATION_DELIVERY_ID_LENGTH,
MAX_NOTIFICATION_MARK_READ_IDS,
insertNotificationDeliveryIfAbsent,
listNotificationDeliveriesForRecipient,
listNotificationSubscriptionsForLogin,
Expand Down Expand Up @@ -228,6 +230,11 @@ describe("notification repository helpers", () => {
await deliverNotification(env, first!.id);
await deliverNotification(env, second!.id);

// Empty ids selects no specific notifications.
expect(await markNotificationDeliveriesRead(env, "miner", [])).toBe(0);
expect((await getNotificationDeliveryById(env, first!.id))?.status).toBe("delivered");
expect((await getNotificationDeliveryById(env, second!.id))?.status).toBe("delivered");

// Mark only the first by id.
expect(await markNotificationDeliveriesRead(env, "miner", [first!.id])).toBe(1);
expect((await getNotificationDeliveryById(env, first!.id))?.status).toBe("read");
Expand All @@ -241,6 +248,20 @@ describe("notification repository helpers", () => {
expect(feed.unreadCount).toBe(0);
});

it("rejects oversized mark-read id filters before building SQL", async () => {
const env = createTestEnv();
await expect(
markNotificationDeliveriesRead(
env,
"miner",
Array.from({ length: MAX_NOTIFICATION_MARK_READ_IDS + 1 }, (_, index) => `id-${index}`),
),
).rejects.toThrow(/at most/);
await expect(markNotificationDeliveriesRead(env, "miner", ["x".repeat(MAX_NOTIFICATION_DELIVERY_ID_LENGTH + 1)])).rejects.toThrow(
/at most/,
);
});

it("returns null for an unknown delivery id", async () => {
const env = createTestEnv();
expect(await getNotificationDeliveryById(env, "missing")).toBeNull();
Expand Down