chore(comments): address PR #143 review (operator doc, badge test, dedup, type)

- [warn 1] Document the is_agent operator setup so it survives plan deletion:
  added an AI-agent block to .env.example (use a DEDICATED account, set is_agent
  via SQL, never flag a human/shared account) + a CHANGELOG "Added" entry.
- [warn 2] Test the badge deep-link side effects: ai-agent-badge.test.tsx now
  renders inside an explicit jotai store, clicks the badge, and asserts the
  active chat id, window-open, cleared draft, closed history modal, AND that
  stopPropagation keeps a parent onClick from firing.
- [suggestion 3] Hoist the window.matchMedia stub into vitest.setup.ts and drop
  the duplicated beforeAll block from the three test files (ai-agent-badge,
  comment-list-item, role-cards).
- [suggestion 4] Merge the two near-duplicate "non-clickable" cases via it.each.
- [follow-up 6] Introduce a single ProvenanceSource = 'user' | 'agent' type in
  jwt-payload.ts and reference it from AuthProvenanceData, JwtPayload/
  JwtCollabPayload, and resolveSource() — so a typo can't slip through as a bare
  string. (Server auth chain; client IComment mirroring left as a follow-up.)

Follow-up 5 (shared agentSourceFields write-stamp helper) is deferred as the
review marked it — the 6 REST sites use varied shapes (create-spread vs
resolve-conditional-null vs page move), so it's a separate focused refactor.

Tests: client badge/comment/role-cards suites 11/11 pass; server auth+comment
suites 62 pass; typecheck clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-23 23:56:26 +03:00
parent 989f99abae
commit 0647faefcd
9 changed files with 94 additions and 70 deletions

View File

@@ -123,6 +123,14 @@ MCP_DOCMOST_PASSWORD=
# expose the port publicly).
# MCP_TOKEN=
# MCP_SESSION_IDLE_MS=1800000
#
# AI-AGENT ATTRIBUTION (comments/pages written via MCP are badged as "AI"):
# attribution is driven by a per-user `is_agent` flag on the users row. There is
# NO admin UI/API for it — set it out-of-band with SQL. Use a DEDICATED service
# account for the MCP fallback above and flag ONLY that account, e.g.:
# UPDATE users SET is_agent = true WHERE email = 'mcp-bot@your-domain';
# NEVER set is_agent on a human or shared account — every action by that account
# (including normal human edits) would then be mis-attributed as AI.
# Per-embedding-call timeout in milliseconds for the RAG indexer.
# A slow/hung embeddings endpoint fails after this and the batch continues.

View File

@@ -10,6 +10,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Added
- **AI-agent attribution for MCP writes.** Comments (and pages) created through
the MCP endpoint by a dedicated agent account are now badged as "AI", with
unspoofable provenance derived from a per-user `is_agent` flag (not from the
request body). **Operator setup:** use a *dedicated* service account for the
MCP fallback and set the flag with SQL —
`UPDATE users SET is_agent = true WHERE email = '<mcp-account>'`. Never flag a
human or shared account, or its normal edits get mis-attributed as AI. See the
AI-agent block in `.env.example`. (#143)
### Changed
- **Public share AI: default per-workspace hourly assistant cap lowered

View File

@@ -1,25 +1,16 @@
import { describe, it, expect, vi, beforeAll } from "vitest";
import { describe, it, expect, vi } from "vitest";
import { render, screen, fireEvent } from "@testing-library/react";
import { MantineProvider } from "@mantine/core";
import { Provider, createStore } from "jotai";
import { AiAgentBadge } from "./ai-agent-badge";
import {
activeAiChatIdAtom,
aiChatWindowOpenAtom,
aiChatDraftAtom,
} from "@/features/ai-chat/atoms/ai-chat-atom.ts";
import { historyAtoms } from "@/features/page-history/atoms/history-atoms.ts";
// MantineProvider reads window.matchMedia (color scheme) on mount, which jsdom
// does not implement. Provide a minimal stub so the provider can render.
beforeAll(() => {
Object.defineProperty(window, "matchMedia", {
writable: true,
value: (query: string) => ({
matches: false,
media: query,
onchange: null,
addListener: vi.fn(),
removeListener: vi.fn(),
addEventListener: vi.fn(),
removeEventListener: vi.fn(),
dispatchEvent: vi.fn(),
}),
});
});
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
function renderBadge(props: { authorName?: string; aiChatId?: string | null }) {
return render(
@@ -40,21 +31,42 @@ describe("AiAgentBadge", () => {
const badge = screen.getByRole("button");
expect(badge).toBeDefined();
expect(badge.textContent).toContain("AI-agent");
// Clicking does not throw — the deep-link handler runs against the default
// jotai store. (Asserting the badge exposes an interactive role is the
// observable contract; the atom side-effects are covered by the history UI.)
fireEvent.click(badge);
});
it("is a plain non-clickable label when aiChatId is null (external MCP agent)", () => {
renderBadge({ authorName: "Bot", aiChatId: null });
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(
<Provider store={store}>
<MantineProvider>
{/* Parent click handler must NOT fire — the badge stops propagation. */}
<div onClick={onParentClick}>
<AiAgentBadge authorName="Bot" aiChatId="chat-1" />
</div>
</MantineProvider>
</Provider>,
);
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
expect(onParentClick).not.toHaveBeenCalled(); // stopPropagation contained the click
});
it.each([{ aiChatId: null }, {}])(
"is a plain non-clickable label without a chat target (%o)",
(props) => {
renderBadge({ authorName: "Bot", ...props });
expect(screen.getByText("AI-agent")).toBeDefined();
// No interactive role is exposed when there is no chat to deep-link into.
expect(screen.queryByRole("button")).toBeNull();
});
it("is non-clickable when aiChatId is absent", () => {
renderBadge({ authorName: "Bot" });
expect(screen.queryByRole("button")).toBeNull();
});
},
);
});

View File

@@ -1,26 +1,10 @@
import { describe, it, expect, vi, beforeAll } from "vitest";
import { describe, it, expect, vi } from "vitest";
import { render, screen, fireEvent } from "@testing-library/react";
import { MantineProvider } from "@mantine/core";
import RoleCards from "./role-cards";
import { IAiRole } from "@/features/ai-chat/types/ai-chat.types.ts";
// MantineProvider reads window.matchMedia (color scheme) on mount, which jsdom
// does not implement. Provide a minimal stub so the provider can render.
beforeAll(() => {
Object.defineProperty(window, "matchMedia", {
writable: true,
value: (query: string) => ({
matches: false,
media: query,
onchange: null,
addListener: vi.fn(),
removeListener: vi.fn(),
addEventListener: vi.fn(),
removeEventListener: vi.fn(),
dispatchEvent: vi.fn(),
}),
});
});
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
const roles: IAiRole[] = [
{

View File

@@ -1,24 +1,9 @@
import { describe, it, expect, vi, beforeAll } from "vitest";
import { describe, it, expect, vi } from "vitest";
import { render, screen } from "@testing-library/react";
import { MantineProvider } from "@mantine/core";
import { IComment } from "@/features/comment/types/comment.types";
// MantineProvider reads window.matchMedia on mount, which jsdom lacks.
beforeAll(() => {
Object.defineProperty(window, "matchMedia", {
writable: true,
value: (query: string) => ({
matches: false,
media: query,
onchange: null,
addListener: vi.fn(),
removeListener: vi.fn(),
addEventListener: vi.fn(),
removeEventListener: vi.fn(),
dispatchEvent: vi.fn(),
}),
});
});
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
// The comment mutation hooks reach out to react-query/network — stub them so the
// component renders in isolation. We only assert the AI-badge rendering branch.

View File

@@ -49,3 +49,17 @@ function createStorage(): Storage {
// `window.localStorage` resolve to the same working stub.
vi.stubGlobal("localStorage", createStorage());
vi.stubGlobal("sessionStorage", createStorage());
// MantineProvider (and other components) read `window.matchMedia` on mount, which
// jsdom does not implement. Provide a minimal stub here so any test rendering
// Mantine works without re-stubbing matchMedia in every file.
vi.stubGlobal("matchMedia", (query: string) => ({
matches: false,
media: query,
onchange: null,
addListener: vi.fn(),
removeListener: vi.fn(),
addEventListener: vi.fn(),
removeEventListener: vi.fn(),
dispatchEvent: vi.fn(),
}));

View File

@@ -21,6 +21,7 @@ import { KyselyDB } from '@docmost/db/types/kysely.types';
import { executeTx } from '@docmost/db/utils';
import { InjectQueue } from '@nestjs/bullmq';
import { QueueJob, QueueName } from '../../integrations/queue/constants';
import { ProvenanceSource } from '../../core/auth/dto/jwt-payload';
import { Queue } from 'bullmq';
import {
extractMentions,
@@ -50,7 +51,7 @@ import { TransclusionService } from '../../core/page/transclusion/transclusion.s
export function resolveSource(
stickyTouched: boolean,
contextActor?: string,
): 'agent' | 'user' {
): ProvenanceSource {
return stickyTouched || contextActor === 'agent' ? 'agent' : 'user';
}

View File

@@ -1,4 +1,5 @@
import { createParamDecorator, ExecutionContext } from '@nestjs/common';
import { ProvenanceSource } from '../../core/auth/dto/jwt-payload';
/**
* The agent-edit provenance carried by the request, read from the SIGNED access
@@ -8,7 +9,7 @@ import { createParamDecorator, ExecutionContext } from '@nestjs/common';
* cannot fake an 'agent' marker.
*/
export interface AuthProvenanceData {
actor: 'user' | 'agent';
actor: ProvenanceSource;
aiChatId: string | null;
}

View File

@@ -1,3 +1,11 @@
/**
* Provenance actor for a write: who the action is attributed to. Derived only
* from the SIGNED token claim (never a request body), so 'agent' is unspoofable.
* Single source of truth so a typo like 'agnet' can't slip through as a bare
* string (#143 review). Distinct from `ActorType` (auth principal kind).
*/
export type ProvenanceSource = 'user' | 'agent';
export enum JwtType {
ACCESS = 'access',
COLLAB = 'collab',
@@ -19,7 +27,7 @@ export type JwtPayload = {
// mints a provenance access token so REST writes (create/rename/move page,
// comment create/resolve) record a non-spoofable 'agent' marker (§6.5 / §15
// C3 / §14 N2).
actor?: 'user' | 'agent';
actor?: ProvenanceSource;
// Nullable: an external MCP agent has no internal ai_chats row, so it carries
// an 'agent' actor with a null aiChatId.
aiChatId?: string | null;
@@ -32,7 +40,7 @@ export type JwtCollabPayload = {
// Optional agent-edit provenance, signed into the collab token. Absent for
// the human collab path (treated as 'user'); set only when the internal agent
// mints a provenance collab token (§6.6 / §15 C2).
actor?: 'user' | 'agent';
actor?: ProvenanceSource;
// Nullable: an external MCP agent has no internal ai_chats row, so it carries
// an 'agent' actor with a null aiChatId.
aiChatId?: string | null;