fix(#300 review): drop stray symlink, re-fetch enriched on comment update, cover history mapping (F1/F2/F3)

F1: remove an accidentally-committed self-referential symlink packages/mcp/node_modules/node_modules -> an absolute build-machine path (leaked a dev home path, a pnpm artifact useless in the repo), and add a targeted ignore so it can't recommit. F2: the commentUpdated broadcast re-emitted the caller's pre-loaded comment mutated in place, so the {agent,launcher} stack survived only because the controller happened to load it with includeCreator:true — the fragile coupling that let the stack vanish on edit once already. update() now RE-FETCHES the enriched comment before broadcasting, symmetric with create()/resolveComment() (the row is already persisted), so all three broadcasts carry the stack regardless of any caller's pre-load. Adds a caller-contract test asserting all three broadcasts emit agent/launcher for an agent comment and neither for a non-agent one, spotlighting the update path (non-vacuous vs the old re-emit). F3: add a direct test of the page-history attachPageHistoryAgent mapping (its distinct lastUpdatedSource/lastUpdatedAiChatId/lastUpdatedBy column set): role / no-role / MCP / non-agent, and that the internal agentRole join column is stripped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-03 06:38:25 +03:00
parent 0968ea97d2
commit 86c1307ed2
5 changed files with 360 additions and 6 deletions
@@ -0,0 +1,107 @@
+import { PageHistoryRepo } from './page-history.repo';
+
+/**
+ * Enrichment coverage for the page-history agent avatar stack (#300/#304).
+ *
+ * attachPageHistoryAgent maps a DIFFERENT column set than comments —
+ * `lastUpdatedSource` / `lastUpdatedAiChatId` / `lastUpdatedBy` instead of
+ * `createdSource` / `aiChatId` / `creator` — so it needs its own direct proof
+ * that the {agent,launcher} pair resolves for each provenance shape and that the
+ * internal `agentRole` join column is stripped.
+ *
+ * The mapping is exercised through findPageHistoryByPageId (the only page-history
+ * path that enriches). The Kysely db is a chainable recorder: query-builder
+ * methods return the builder and `.execute()` (called by
+ * executeWithCursorPagination) yields preset rows, so no real database is
+ * touched. The `.select((eb) => ...)` callbacks are recorded but never invoked,
+ * so the preset row stands in for what the DB would have returned.
+ *
+ * NON-VACUITY: against an identity mapping (raw row pass-through) the agent-case
+ * assertions fail — `agent`/`launcher` would be undefined and the internal
+ * `agentRole` column would leak.
+ */
+describe('PageHistoryRepo.findPageHistoryByPageId — agent avatar stack enrichment', () => {
+  function makeRepo(rows: unknown[]) {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const builder: any = {
+      selectFrom: () => builder,
+      select: () => builder,
+      where: () => builder,
+      orderBy: () => builder,
+      limit: () => builder,
+      execute: async () => rows,
+    };
+    const db = { selectFrom: () => builder };
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    return new PageHistoryRepo(db as any);
+  }
+
+  // perPage high enough that a single preset row never triggers the extra-row
+  // "has next page" branch (which would call generateCursor).
+  const pagination = { limit: 50 } as any;
+
+  const firstItem = async (row: Record<string, unknown>) => {
+    const repo = makeRepo([row]);
+    const result = await repo.findPageHistoryByPageId('page-1', pagination);
+    return result.items[0] as any;
+  };
+
+  it('internal chat WITH role: agent = role (emoji, no avatar), launcher = human, agentRole stripped', async () => {
+    const item = await firstItem({
+      id: 'ph-1',
+      lastUpdatedSource: 'agent',
+      lastUpdatedAiChatId: 'chat-1',
+      lastUpdatedBy: { name: 'Alice', avatarUrl: 'a.png' },
+      agentRole: { name: 'Editor', emoji: '✏️' },
+    });
+
+    expect(item.agent).toEqual({ name: 'Editor', emoji: '✏️', avatarUrl: null });
+    expect(item.launcher).toEqual({ name: 'Alice', avatarUrl: 'a.png' });
+    // The internal join column must never leak to the client.
+    expect(item).not.toHaveProperty('agentRole');
+  });
+
+  it('internal chat WITHOUT role: agent = "AI agent" fallback, launcher = human', async () => {
+    const item = await firstItem({
+      id: 'ph-2',
+      lastUpdatedSource: 'agent',
+      lastUpdatedAiChatId: 'chat-1',
+      lastUpdatedBy: { name: 'Alice', avatarUrl: 'a.png' },
+      agentRole: null,
+    });
+
+    expect(item.agent).toEqual({ name: 'AI agent', avatarUrl: null });
+    expect(item.agent).not.toHaveProperty('emoji');
+    expect(item.launcher).toEqual({ name: 'Alice', avatarUrl: 'a.png' });
+    expect(item).not.toHaveProperty('agentRole');
+  });
+
+  it('external MCP (lastUpdatedAiChatId null): agent = the account itself, launcher = null', async () => {
+    const item = await firstItem({
+      id: 'ph-3',
+      lastUpdatedSource: 'agent',
+      lastUpdatedAiChatId: null,
+      lastUpdatedBy: { name: 'MCP Bot', avatarUrl: 'bot.png' },
+      agentRole: null,
+    });
+
+    expect(item.agent).toEqual({ name: 'MCP Bot', avatarUrl: 'bot.png' });
+    expect(item.launcher).toBeNull();
+    expect(item).not.toHaveProperty('agentRole');
+  });
+
+  it('non-agent (lastUpdatedSource !== "agent"): neither agent nor launcher, agentRole stripped', async () => {
+    const item = await firstItem({
+      id: 'ph-4',
+      lastUpdatedSource: 'user',
+      lastUpdatedAiChatId: null,
+      lastUpdatedBy: { name: 'Bob', avatarUrl: null },
+      agentRole: null,
+    });
+
+    expect(item).not.toHaveProperty('agent');
+    expect(item).not.toHaveProperty('launcher');
+    // A plain human row still strips the internal join column.
+    expect(item).not.toHaveProperty('agentRole');
+  });
+});