diff --git a/apps/client/src/components/ui/ai-agent-badge.test.tsx b/apps/client/src/components/ui/ai-agent-badge.test.tsx
index 8932d0a2..678013ed 100644
--- a/apps/client/src/components/ui/ai-agent-badge.test.tsx
+++ b/apps/client/src/components/ui/ai-agent-badge.test.tsx
@@ -8,7 +8,6 @@ import {
aiChatWindowOpenAtom,
aiChatDraftAtom,
} from "@/features/ai-chat/atoms/ai-chat-atom.ts";
-import { historyAtoms } from "@/features/page-history/atoms/history-atoms.ts";
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
@@ -20,6 +19,33 @@ function renderBadge(props: { authorName?: string; aiChatId?: string | null }) {
);
}
+// Render a clickable badge inside an explicit jotai store, with a leftover draft
+// and an onActivate + parent-click spy, so the deep-link side effects are
+// assertable. Returns the store and spies.
+function setupClickable() {
+ const store = createStore();
+ store.set(aiChatDraftAtom, "leftover draft from another chat");
+ const onActivate = vi.fn();
+ const onParentClick = vi.fn();
+ render(
+
+
+
+
+ ,
+ );
+ return { store, onActivate, onParentClick, badge: screen.getByRole("button") };
+}
+
+function expectDeepLinked(store: ReturnType, onActivate: ReturnType) {
+ expect(store.get(activeAiChatIdAtom)).toBe("chat-1");
+ expect(store.get(aiChatWindowOpenAtom)).toBe(true);
+ expect(store.get(aiChatDraftAtom)).toBe(""); // draft cleared
+ expect(onActivate).toHaveBeenCalledTimes(1); // caller closes its own modal etc.
+}
+
describe("AiAgentBadge", () => {
it("renders the AI-agent label", () => {
renderBadge({ authorName: "Bot" });
@@ -33,33 +59,31 @@ describe("AiAgentBadge", () => {
expect(badge.textContent).toContain("AI-agent");
});
- it("deep-links on click: sets the active chat, clears the draft, opens the AI-chat window, closes the history modal — and stops propagation", () => {
- const store = createStore();
- // Pre-set the state the click must change, so the assertions are meaningful.
- store.set(historyAtoms, true); // history modal open
- store.set(aiChatDraftAtom, "leftover draft from another chat");
- const onParentClick = vi.fn();
-
- render(
-
-
- {/* Parent click handler must NOT fire — the badge stops propagation. */}
-
-
- ,
- );
-
- fireEvent.click(screen.getByRole("button"));
-
- expect(store.get(activeAiChatIdAtom)).toBe("chat-1");
- expect(store.get(aiChatWindowOpenAtom)).toBe(true);
- expect(store.get(aiChatDraftAtom)).toBe(""); // draft cleared
- expect(store.get(historyAtoms)).toBe(false); // history modal closed
+ it("click deep-links: sets active chat, clears draft, opens window, fires onActivate, stops propagation", () => {
+ const { store, onActivate, onParentClick, badge } = setupClickable();
+ fireEvent.click(badge);
+ expectDeepLinked(store, onActivate);
expect(onParentClick).not.toHaveBeenCalled(); // stopPropagation contained the click
});
+ it.each(["Enter", " "])(
+ "keyboard %j activates the deep-link (same side effects as click)",
+ (key) => {
+ const { store, onActivate, badge } = setupClickable();
+ fireEvent.keyDown(badge, { key });
+ expectDeepLinked(store, onActivate);
+ },
+ );
+
+ it("an unrelated key does NOT activate the badge", () => {
+ const { store, onActivate, badge } = setupClickable();
+ fireEvent.keyDown(badge, { key: "Tab" });
+ expect(store.get(activeAiChatIdAtom)).toBeNull();
+ expect(store.get(aiChatWindowOpenAtom)).toBe(false);
+ expect(store.get(aiChatDraftAtom)).toBe("leftover draft from another chat");
+ expect(onActivate).not.toHaveBeenCalled();
+ });
+
it.each([{ aiChatId: null }, {}])(
"is a plain non-clickable label without a chat target (%o)",
(props) => {
diff --git a/apps/client/src/components/ui/ai-agent-badge.tsx b/apps/client/src/components/ui/ai-agent-badge.tsx
index e7879177..39e29614 100644
--- a/apps/client/src/components/ui/ai-agent-badge.tsx
+++ b/apps/client/src/components/ui/ai-agent-badge.tsx
@@ -8,11 +8,14 @@ import {
aiChatWindowOpenAtom,
aiChatDraftAtom,
} from "@/features/ai-chat/atoms/ai-chat-atom.ts";
-import { historyAtoms } from "@/features/page-history/atoms/history-atoms.ts";
interface AiAgentBadgeProps {
authorName?: string;
aiChatId?: string | null;
+ // Fired after the badge deep-links into its chat. The caller handles its own
+ // context (e.g. the page-history row closes the history modal) so this generic
+ // ui/ primitive stays free of cross-feature coupling (#143 review Arch B).
+ onActivate?: () => void;
}
/**
@@ -21,18 +24,22 @@ interface AiAgentBadgeProps {
* page-history list and the comments sidebar.
*
* When the item carries an `aiChatId` (an internal AI-chat edit), clicking the
- * badge deep-links into that chat: it sets the active-chat atom, opens the
- * floating AI-chat window, and closes the history modal. When `aiChatId` is
- * null/absent (an external MCP write with no internal ai_chats row), the badge
- * is a plain non-clickable label. The click is contained (stopPropagation) so it
- * does not also trigger an enclosing row's click handler.
+ * badge deep-links into that chat: it sets the active-chat atom and opens the
+ * floating AI-chat window, then invokes `onActivate` so the caller can react
+ * (e.g. the history modal closes itself). When `aiChatId` is null/absent (an
+ * external MCP write with no internal ai_chats row), the badge is a plain
+ * non-clickable label. The click is contained (stopPropagation) so it does not
+ * also trigger an enclosing row's click handler.
*/
-export function AiAgentBadge({ authorName, aiChatId }: AiAgentBadgeProps) {
+export function AiAgentBadge({
+ authorName,
+ aiChatId,
+ onActivate,
+}: AiAgentBadgeProps) {
const { t } = useTranslation();
const setAiChatWindowOpen = useSetAtom(aiChatWindowOpenAtom);
const setActiveChatId = useSetAtom(activeAiChatIdAtom);
const setDraft = useSetAtom(aiChatDraftAtom);
- const setHistoryModalOpen = useSetAtom(historyAtoms);
const tooltip = t("Edited by AI agent on behalf of {{name}}", {
name: authorName ?? "",
@@ -47,9 +54,9 @@ export function AiAgentBadge({ authorName, aiChatId }: AiAgentBadgeProps) {
// unsent draft so it does not leak from the previously open chat.
setDraft("");
setAiChatWindowOpen(true);
- setHistoryModalOpen(false);
+ onActivate?.();
},
- [aiChatId, setActiveChatId, setDraft, setAiChatWindowOpen, setHistoryModalOpen],
+ [aiChatId, setActiveChatId, setDraft, setAiChatWindowOpen, onActivate],
);
const badge = (
diff --git a/apps/client/src/features/comment/components/comment-list-item.test.tsx b/apps/client/src/features/comment/components/comment-list-item.test.tsx
index 53796bc9..82e12785 100644
--- a/apps/client/src/features/comment/components/comment-list-item.test.tsx
+++ b/apps/client/src/features/comment/components/comment-list-item.test.tsx
@@ -53,10 +53,7 @@ describe("CommentListItem — AI badge", () => {
expect(screen.getByText("Service Bot")).toBeDefined();
});
- it("renders a non-clickable badge when aiChatId is null (external MCP agent)", () => {
- renderItem(baseComment({ createdSource: "agent", aiChatId: null }));
- expect(screen.getByText("AI-agent")).toBeDefined();
- // No deep-link target → no interactive button role.
- expect(screen.queryByRole("button")).toBeNull();
- });
+ // The non-clickable (null aiChatId) branch is a property of AiAgentBadge itself
+ // and is covered in ai-agent-badge.test.tsx; this integration suite only needs
+ // the insertion gate (agent → badge, user → no badge) above (#143 review).
});
diff --git a/apps/client/src/features/page-history/components/history-item.tsx b/apps/client/src/features/page-history/components/history-item.tsx
index 83f6457e..ccb15c0a 100644
--- a/apps/client/src/features/page-history/components/history-item.tsx
+++ b/apps/client/src/features/page-history/components/history-item.tsx
@@ -6,6 +6,8 @@ import classes from "./css/history.module.css";
import clsx from "clsx";
import { IPageHistory } from "@/features/page-history/types/page.types";
import { memo, useCallback } from "react";
+import { useSetAtom } from "jotai";
+import { historyAtoms } from "@/features/page-history/atoms/history-atoms.ts";
const MAX_VISIBLE_AVATARS = 5;
@@ -26,6 +28,8 @@ const HistoryItem = memo(function HistoryItem({
onHoverEnd,
isActive,
}: HistoryItemProps) {
+ const setHistoryModalOpen = useSetAtom(historyAtoms);
+
const handleClick = useCallback(() => {
onSelect(historyItem.id, index);
}, [onSelect, historyItem.id, index]);
@@ -99,6 +103,9 @@ const HistoryItem = memo(function HistoryItem({
setHistoryModalOpen(false)}
/>
)}
diff --git a/apps/server/src/collaboration/extensions/authentication.extension.spec.ts b/apps/server/src/collaboration/extensions/authentication.extension.spec.ts
index 19c727ec..1871d09a 100644
--- a/apps/server/src/collaboration/extensions/authentication.extension.spec.ts
+++ b/apps/server/src/collaboration/extensions/authentication.extension.spec.ts
@@ -208,4 +208,19 @@ describe('AuthenticationExtension.onAuthenticate', () => {
expect(ctx.actor).toBe('user');
expect(ctx.aiChatId).toBeNull();
});
+
+ it('is_agent user with NO claim → actor=agent (collab seam consults the signed identity)', async () => {
+ // Arch A regression guard: a flagged service account editing page CONTENT
+ // over the collab websocket carries a plain COLLAB token (no actor claim).
+ // Before the shared resolveProvenance() wiring this seam derived actor from
+ // the claim alone, so such edits persisted as lastUpdatedSource='user' —
+ // drifting from the REST seam. The seam must now stamp 'agent' from the
+ // is_agent flag, matching jwt.strategy.
+ userRepo.findById.mockResolvedValue(buildUser({ isAgent: true }));
+ const ctx = await ext.onAuthenticate(buildData() as any);
+
+ expect(ctx.actor).toBe('agent');
+ // No internal ai_chats row for an MCP/service-account collab edit → null.
+ expect(ctx.aiChatId).toBeNull();
+ });
});
diff --git a/apps/server/src/collaboration/extensions/authentication.extension.ts b/apps/server/src/collaboration/extensions/authentication.extension.ts
index 4bfe67ca..41646c12 100644
--- a/apps/server/src/collaboration/extensions/authentication.extension.ts
+++ b/apps/server/src/collaboration/extensions/authentication.extension.ts
@@ -15,6 +15,7 @@ import { SpaceRole } from '../../common/helpers/types/permission';
import { isUserDisabled } from '../../common/helpers';
import { getPageId } from '../collaboration.util';
import { JwtCollabPayload, JwtType } from '../../core/auth/dto/jwt-payload';
+import { resolveProvenance } from '../../common/decorators/auth-provenance.decorator';
@Injectable()
export class AuthenticationExtension implements Extension {
@@ -103,13 +104,17 @@ export class AuthenticationExtension implements Extension {
this.logger.debug(`Authenticated user ${user.id} on page ${pageId}`);
- // Carry the signed agent-edit provenance claim into the hocuspocus
- // connection context (§6.6 / §15 C2). The human collab path omits these
- // claims, so it resolves to actor='user' / aiChatId=null.
+ // Carry the agent-edit provenance into the hocuspocus connection context
+ // (§6.6 / §15 C2), derived via the SAME resolver as the REST seam so the two
+ // can't drift. An is_agent service account (e.g. the MCP bot) is attributed
+ // 'agent' here too, so its page-content edits over collab persist as
+ // lastUpdatedSource='agent' (#143 review Arch A) — not just its REST writes.
+ // The human collab path carries no claim and is not flagged → actor='user'.
+ const provenance = resolveProvenance(user, jwtPayload);
return {
user,
- actor: jwtPayload.actor ?? 'user',
- aiChatId: jwtPayload.aiChatId ?? null,
+ actor: provenance.actor,
+ aiChatId: provenance.aiChatId,
};
}
}
diff --git a/apps/server/src/common/decorators/auth-provenance.decorator.spec.ts b/apps/server/src/common/decorators/auth-provenance.decorator.spec.ts
new file mode 100644
index 00000000..99d7341f
--- /dev/null
+++ b/apps/server/src/common/decorators/auth-provenance.decorator.spec.ts
@@ -0,0 +1,91 @@
+import {
+ resolveProvenance,
+ agentSourceFields,
+} from './auth-provenance.decorator';
+
+/**
+ * Unit tests for the shared provenance helpers (#143 review, Arch A & follow-up
+ * 5). resolveProvenance is the single source of truth wired into BOTH transport
+ * seams (REST jwt.strategy + collab authentication.extension) — testing it here
+ * pins the derivation matrix so the seams can't silently drift. agentSourceFields
+ * is the one-place write-stamp idiom reused at every insert/update site.
+ */
+describe('resolveProvenance', () => {
+ it("flags an is_agent user as 'agent' even with no claim (the closed collab gap)", () => {
+ expect(resolveProvenance({ isAgent: true }, undefined)).toEqual({
+ actor: 'agent',
+ aiChatId: null,
+ });
+ });
+
+ it("an is_agent user keeps the claim's aiChatId when present", () => {
+ expect(
+ resolveProvenance({ isAgent: true }, { aiChatId: 'chat-1' }),
+ ).toEqual({ actor: 'agent', aiChatId: 'chat-1' });
+ });
+
+ it("honors a signed actor='agent' claim on a non-agent user (internal AI-chat token)", () => {
+ expect(
+ resolveProvenance(
+ { isAgent: false },
+ { actor: 'agent', aiChatId: 'chat-2' },
+ ),
+ ).toEqual({ actor: 'agent', aiChatId: 'chat-2' });
+ });
+
+ it("a plain user with no claim resolves to 'user' with null chat", () => {
+ expect(resolveProvenance({ isAgent: false }, undefined)).toEqual({
+ actor: 'user',
+ aiChatId: null,
+ });
+ });
+
+ it('tolerates a null/undefined user (defaults to the claim, else user)', () => {
+ expect(resolveProvenance(null, null)).toEqual({
+ actor: 'user',
+ aiChatId: null,
+ });
+ expect(resolveProvenance(undefined, { actor: 'agent' })).toEqual({
+ actor: 'agent',
+ aiChatId: null,
+ });
+ });
+});
+
+describe('agentSourceFields', () => {
+ it('stamps the configured source + chat columns for an agent write', () => {
+ expect(
+ agentSourceFields(
+ { actor: 'agent', aiChatId: 'chat-1' },
+ 'createdSource',
+ 'aiChatId',
+ ),
+ ).toEqual({ createdSource: 'agent', aiChatId: 'chat-1' });
+ });
+
+ it('uses the per-table column names passed in (page update variant)', () => {
+ expect(
+ agentSourceFields(
+ { actor: 'agent', aiChatId: null },
+ 'lastUpdatedSource',
+ 'lastUpdatedAiChatId',
+ ),
+ ).toEqual({ lastUpdatedSource: 'agent', lastUpdatedAiChatId: null });
+ });
+
+ it('returns {} for a user write so the column keeps its default', () => {
+ expect(
+ agentSourceFields(
+ { actor: 'user', aiChatId: null },
+ 'createdSource',
+ 'aiChatId',
+ ),
+ ).toEqual({});
+ });
+
+ it('returns {} when provenance is undefined', () => {
+ expect(
+ agentSourceFields(undefined, 'createdSource', 'aiChatId'),
+ ).toEqual({});
+ });
+});
diff --git a/apps/server/src/common/decorators/auth-provenance.decorator.ts b/apps/server/src/common/decorators/auth-provenance.decorator.ts
index 8baa592b..e4be9d20 100644
--- a/apps/server/src/common/decorators/auth-provenance.decorator.ts
+++ b/apps/server/src/common/decorators/auth-provenance.decorator.ts
@@ -13,6 +13,30 @@ export interface AuthProvenanceData {
aiChatId: string | null;
}
+/**
+ * Single source of truth for deriving a write's provenance from the SIGNED
+ * server-side identity (#143 review, Arch A). Used by BOTH transport seams — the
+ * REST access-token strategy and the collab websocket auth — so they can't drift:
+ *
+ * - A `user.isAgent` service account (e.g. the MCP bot) stamps 'agent' on every
+ * write. It has no internal ai_chats row, so aiChatId comes from the claim
+ * (usually null).
+ * - Otherwise honor the actor claim minted into the internal AI agent's token
+ * (actor='agent' + aiChatId); a normal user token carries no claim → 'user'.
+ *
+ * Provenance is NEVER read from a client body field, so a normal user cannot fake
+ * an 'agent' marker.
+ */
+export function resolveProvenance(
+ user: { isAgent?: boolean | null } | null | undefined,
+ claim: { actor?: ProvenanceSource; aiChatId?: string | null } | null | undefined,
+): AuthProvenanceData {
+ const actor: ProvenanceSource = user?.isAgent
+ ? 'agent'
+ : (claim?.actor ?? 'user');
+ return { actor, aiChatId: claim?.aiChatId ?? null };
+}
+
/**
* Agent-edit write-stamp fields for a repository insert/update (#143 review).
* Spread into the row being written: for an agent it stamps the `*Source`
diff --git a/apps/server/src/core/auth/strategies/jwt.strategy.spec.ts b/apps/server/src/core/auth/strategies/jwt.strategy.spec.ts
index 2e131e92..ac0e2c08 100644
--- a/apps/server/src/core/auth/strategies/jwt.strategy.spec.ts
+++ b/apps/server/src/core/auth/strategies/jwt.strategy.spec.ts
@@ -77,31 +77,20 @@ describe('JwtStrategy — provenance derivation', () => {
expect(req.raw.aiChatId).toBeNull();
});
- it("does NOT let an 'actor' claim escalate a non-agent user beyond the existing claim semantics", async () => {
- // A non-agent user. The only way the token carries actor='agent' is the
- // internal AI-chat's server-minted token (the claim cannot be set by a
- // client on a plain login). We assert the derivation falls back to the
- // claim ONLY when is_agent is false — i.e. an is_agent=false user is never
- // forced to 'agent' by anything other than that signed claim, and a plain
- // user (no claim) stays 'user'.
+ it("honors a SIGNED actor='agent' claim on a non-agent user's token (the internal AI-chat path)", async () => {
+ // A non-agent user (the plain no-claim → 'user' case is covered above). A
+ // token that DOES carry actor='agent' resolves to 'agent' — BY DESIGN: that
+ // claim can only exist on a SERVER-MINTED provenance token (the internal AI
+ // chat), never on a plain login token, because the token is signed with the
+ // app secret. The guarantee is that a client cannot FORGE this signed claim,
+ // not that the strategy ignores it. (A plain user still cannot obtain
+ // 'agent' — they have no way to get such a token.)
const { strategy } = makeStrategy({
id: 'user-1',
isAgent: false,
deactivatedAt: null,
deletedAt: null,
});
- const req = makeReq();
-
- // No actor claim (the plain-user login case): stays 'user'.
- await strategy.validate(req, accessPayload() as any);
- expect(req.raw.actor).toBe('user');
-
- // A token that DOES carry actor='agent' resolves to 'agent' — BY DESIGN:
- // that claim can only exist on a SERVER-MINTED provenance token (the internal
- // AI chat), never on a plain login token, because the token is signed with
- // the app secret. The security guarantee is that a client cannot forge this
- // signed claim, NOT that the strategy ignores it. (A plain user therefore
- // still cannot obtain 'agent' — they have no way to get such a token.)
const req2 = makeReq();
await strategy.validate(req2, accessPayload({ actor: 'agent', aiChatId: 'chat-1' }) as any);
expect(req2.raw.actor).toBe('agent');
diff --git a/apps/server/src/core/auth/strategies/jwt.strategy.ts b/apps/server/src/core/auth/strategies/jwt.strategy.ts
index 115aee9c..791da95c 100644
--- a/apps/server/src/core/auth/strategies/jwt.strategy.ts
+++ b/apps/server/src/core/auth/strategies/jwt.strategy.ts
@@ -10,6 +10,7 @@ import { SessionActivityService } from '../../session/session-activity.service';
import { FastifyRequest } from 'fastify';
import { extractBearerTokenFromHeader, isUserDisabled } from '../../../common/helpers';
import { ModuleRef } from '@nestjs/core';
+import { resolveProvenance } from '../../../common/decorators/auth-provenance.decorator';
@Injectable()
export class JwtStrategy extends PassportStrategy(Strategy, 'jwt') {
@@ -72,18 +73,14 @@ export class JwtStrategy extends PassportStrategy(Strategy, 'jwt') {
}
// Propagate the agent-edit provenance onto the request so REST
- // services/controllers can set the 'agent' marker off it. Provenance is
- // derived from the SIGNED server-side identity, never from a client body
- // field, so a normal user cannot fake an 'agent' badge:
- // - An account flagged is_agent (an MCP service account) stamps EVERY write
- // as 'agent'. It has no internal ai_chats row, so aiChatId stays null.
- // - Otherwise fall back to the actor claim minted into the internal AI
- // agent's token (actor='agent' + aiChatId); a normal user token carries
- // no claim and resolves to 'user' (unchanged behaviour).
- req.raw.actor = user.isAgent
- ? 'agent'
- : ((payload as JwtPayload).actor ?? 'user');
- req.raw.aiChatId = (payload as JwtPayload).aiChatId ?? null;
+ // services/controllers can set the 'agent' marker off it. Derived from the
+ // SIGNED server-side identity via the shared resolver (also used by the
+ // collab seam, so the two never drift), never from a client body field — so
+ // an is_agent service account stamps every REST write made with an access
+ // token, and a normal user cannot fake an 'agent' badge.
+ const provenance = resolveProvenance(user, payload as JwtPayload);
+ req.raw.actor = provenance.actor;
+ req.raw.aiChatId = provenance.aiChatId;
return { user, workspace };
}
diff --git a/docs/backlog/mcp-comments-ai-attribution.md b/docs/backlog/mcp-comments-ai-attribution.md
index e0ac7607..fbf64de0 100644
--- a/docs/backlog/mcp-comments-ai-attribution.md
+++ b/docs/backlog/mcp-comments-ai-attribution.md
@@ -1,10 +1,11 @@
# Атрибуция комментариев (и записей) от MCP как «AI», а не как пользователь
-Статус: **открыто (дизайн).** Сейчас комментарии, созданные через MCP-инструмент,
-показываются как комментарии обычного пользователя (сервис-аккаунта, под которым
-залогинен MCP). Нужно, чтобы они показывались как комментарии от AI. Инфраструктура
-agent-провенанса (`§15 C3`) в проекте уже наполовину построена — задача переиспользует
-её, а не строит заново.
+Статус: **реализовано (#143).** Комментарии и записи страниц, созданные через MCP
+(или любым `is_agent`-аккаунтом), помечаются неподделываемым AI-бейджем. Провенанс
+выводится из подписанной идентичности на ОБОИХ транспортных швах — REST
+(`jwt.strategy`) и collab-websocket (`authentication.extension`) — через общий
+`resolveProvenance` (см. `auth-provenance.decorator.ts`), поэтому швы не расходятся.
+Документ оставлен как запись дизайна/обоснования; дальнейшая работа по нему не нужна.
## Цель