fix: allow duplicate route IDs across groups and add group_id to send_logs

- Change validateRoutes to per-group ID uniqueness instead of global
- Remove post-merge cross-group route ID uniqueness checks
- Include groupId in msg:* KV key to prevent collision
- Add group_id column to send_logs table (migration 0004)
- Record groupId in send_logs for accurate permission filtering
- Use groupId from log entries for admin log access checks

migrations/0004_add_group_id.sql

@@ -0,0 +1 @@
+ALTER TABLE send_logs ADD COLUMN group_id TEXT;

src/__tests__/send-log.test.ts

@@ -6,6 +6,7 @@ function createMockDB(): D1Database {
   const insertCols = [
     "ts",
     "route_id",
+    "group_id",
     "event",
     "repo",
     "target",

src/core/dispatch.ts

@@ -69,6 +69,7 @@ export async function dispatchEvent(config: Config, event: WebhookEvent, env: En
       const base: {
         ts: number;
         routeId: string;
+        groupId: string | undefined;
         event: string;
         repo: string | undefined;
         target: string;
@@ -78,6 +79,7 @@ export async function dispatchEvent(config: Config, event: WebhookEvent, env: En
       } = {
         ts: Date.now(),
         routeId: route.id,
+        groupId: route.groupId,
         event: event.event,
         repo: (event.payload.repository as { full_name?: string } | undefined)?.full_name,
         target: targetStr,
@@ -91,7 +93,8 @@ export async function dispatchEvent(config: Config, event: WebhookEvent, env: En
         const driver = getDriver(target);
         let result: SendResult;
         if (message.updateKey) {
-          const kvKey = `msg:${route.id}:${message.updateKey}:${targetStr}`;
+          const groupPrefix = route.groupId ? `${route.groupId}:` : "";
+          const kvKey = `msg:${groupPrefix}${route.id}:${message.updateKey}:${targetStr}`;
           const existingId = await env.KV.get(kvKey);
           if (existingId) {
             result = await driver.edit(message, target, env, existingId);

src/lib/send-log.ts

@@ -4,6 +4,7 @@ export interface SendRecord {
   id?: number;
   ts: number;
   routeId: string;
+  groupId?: string;
   event: string;
   repo?: string;
   target: string;
@@ -22,12 +23,13 @@ export interface SendRecord {
 }
 
 const COLUMNS =
-  "id, ts, route_id, event, repo, target, ok, error, status, message_id, delivery_id, platform, actor, action, duration_ms, error_code, attempts, detail";
+  "id, ts, route_id, group_id, event, repo, target, ok, error, status, message_id, delivery_id, platform, actor, action, duration_ms, error_code, attempts, detail";
 
 interface LogRow {
   id: number;
   ts: number;
   route_id: string;
+  group_id: string | null;
   event: string;
   repo: string | null;
   target: string;
@@ -50,6 +52,7 @@ function toRecord(r: LogRow): SendRecord {
     id: r.id,
     ts: r.ts,
     routeId: r.route_id,
+    groupId: r.group_id ?? undefined,
     event: r.event,
     repo: r.repo ?? undefined,
     target: r.target,
@@ -72,12 +75,13 @@ export async function recordSend(db: D1Database, record: SendRecord): Promise<vo
   try {
     await db
       .prepare(
-        `INSERT INTO send_logs (ts, route_id, event, repo, target, ok, error, status, message_id, delivery_id, platform, actor, action, duration_ms, error_code, attempts, detail)
-         VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+        `INSERT INTO send_logs (ts, route_id, group_id, event, repo, target, ok, error, status, message_id, delivery_id, platform, actor, action, duration_ms, error_code, attempts, detail)
+         VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
       )
       .bind(
         record.ts,
         record.routeId,
+        record.groupId ?? null,
         record.event,
         record.repo ?? null,
         record.target,

src/web/admin-routes.ts

@@ -52,15 +52,23 @@ function validateRoutes(
   if (!Array.isArray(routes)) return { ok: false, error: "routes must be an array" };
   if (routes.length > 200) return { ok: false, error: "too many routes" };
 
-  const seen = new Set<string>();
+  const seenByGroup = new Map<string, Set<string>>();
   for (let i = 0; i < routes.length; i++) {
     const r = routes[i] as Record<string, unknown>;
     if (!r || typeof r !== "object") return { ok: false, error: `route[${i}] is not an object` };
     if (typeof r.id !== "string" || !ID_RE.test(r.id)) {
       return { ok: false, error: `route[${i}].id is invalid` };
     }
-    if (seen.has(r.id)) return { ok: false, error: `duplicate route id "${r.id}"` };
-    seen.add(r.id);
+    const gid = (r.groupId as string) ?? "__nogroup__";
+    let groupSeen = seenByGroup.get(gid);
+    if (!groupSeen) {
+      groupSeen = new Set();
+      seenByGroup.set(gid, groupSeen);
+    }
+    if (groupSeen.has(r.id)) {
+      return { ok: false, error: `duplicate route id "${r.id}" in group "${gid}"` };
+    }
+    groupSeen.add(r.id);
     // Skip full validation for routes that are unchanged from what is stored.
     const prev = unchanged?.get(r.id);
     if (prev && deepEqual(r, prev)) continue;
@@ -282,12 +290,8 @@ export function createAdminRoutes(): Hono<{ Bindings: Env }> {
     if (s.scope.isSuper) {
       return c.json({ logs: await getSendLog(c.env.DB, limit) });
     }
-    const all = await loadRoutes(c.env.KV);
-    const allowed = new Set(
-      all.filter((r) => r.groupId != null && s.scope.groupIds.has(r.groupId)).map((r) => r.id),
-    );
     const logs = (await getSendLog(c.env.DB, 200))
-      .filter((l) => allowed.has(l.routeId))
+      .filter((l) => l.groupId != null && s.scope.groupIds.has(l.groupId))
       .slice(0, limit);
     return c.json({ logs });
   });
@@ -300,9 +304,7 @@ export function createAdminRoutes(): Hono<{ Bindings: Env }> {
     const entry = await getSendLogById(c.env.DB, id);
     if (!entry) return c.json({ error: "Log entry not found" }, 404);
     if (!s.scope.isSuper) {
-      const all = await loadRoutes(c.env.KV);
-      const route = all.find((r) => r.id === entry.routeId);
-      if (!route?.groupId || !s.scope.groupIds.has(route.groupId)) {
+      if (!entry.groupId || !s.scope.groupIds.has(entry.groupId)) {
         return c.json({ error: "Forbidden" }, 403);
       }
     }
@@ -344,13 +346,6 @@ export function createAdminRoutes(): Hono<{ Bindings: Env }> {
       ];
     }
 
-    // Guard against duplicate ids across the merged set.
-    const ids = new Set<string>();
-    for (const r of nextAll) {
-      if (ids.has(r.id)) return c.json({ error: `duplicate route id "${r.id}"` }, 400);
-      ids.add(r.id);
-    }
-
     try {
       await saveRoutes(c.env.KV, nextAll);
     } catch (err) {
@@ -422,13 +417,6 @@ export function createAdminRoutes(): Hono<{ Bindings: Env }> {
     const others = existing.filter((r) => r.groupId !== groupId);
     const nextAll = [...others, ...result.routes];
 
-    // Guard against ids colliding with routes in other groups.
-    const ids = new Set<string>();
-    for (const r of nextAll) {
-      if (ids.has(r.id)) return c.json({ error: `duplicate route id "${r.id}"` }, 400);
-      ids.add(r.id);
-    }
-
     try {
       await saveRoutes(c.env.KV, nextAll);
     } catch (err) {