Skip to content

Commit cf9684a

Browse files
Jacob Joveclaude
authored andcommitted
feat(mcp): attach to live Yjs collaboration rooms with attributed tracked changes
Add `superdoc_attach` so an MCP client can join a live SuperDoc Yjs collaboration room over WebSocket (openRoom + WebsocketProvider, awaiting initial sync) instead of only round-tripping a local .docx. The returned session_id works with every existing tool. The motivating use case is agent-assisted review: suggesting tracked redlines into a document a human has open, attributed to a named reviewer, for accept/reject. Tracked-change attribution: `superdoc_attach` accepts an optional user ({ id, name, email }) threaded through openRoom into buildAttachEditor's headless Editor config. Without a configured user, forceTrackChanges rejects tracked edits, so suggested edits over an attach could not be attributed. The file-open path already sets a default user; this brings the attach path to parity, scoped to a caller-supplied identity. Collab-aware save export: a joiner editor is built with no docx source, so converter.convertedXml carried none of the base OOXML parts that Editor.exportDocx dereferences. The deref threw and (via exportDocx's catch) surfaced as "not binary (got undefined)". buildAttachEditor now seeds the blank-docx template via Editor.loadXmlData so export has valid scaffolding; Yjs still drives the body (the initial ProseMirror doc is seeded from `content` only when no ydoc is present). save() rejects room saves without an explicit output path. Sync-wait robustness: the initial-sync wait delegates to the codebase's canonical onCollaborationProviderSynced helper instead of a bespoke on('sync') wait. The helper pre-checks an already-synced provider, listens to both sync(boolean) and the no-arg synced event, and re-checks after wiring to close the register-after-sync race. The default WebsocketProvider was already safe (its sync(true) fires strictly async from websocket.onmessage), but the createProvider seam admits alternate providers — pooled/already-synced, or synced-only emitters — that the bespoke wait would hang on until a spurious timeout. Post-sync editor/adapter construction is now wrapped so a throw there destroys the provider before rethrowing, rather than leaking a live socket, its resync timers, and a process exit handler (this applies to the default provider too). The attach error path also reports non-Error throws as their string form instead of "undefined". Tests: protocol.test.ts now covers superdoc_attach in the tool-list, action-enum, and session_id assertions (like superdoc_open, it creates a session rather than consuming one). New collab-export.test.ts asserts a binary PK-zip round-trip (word/document.xml + styles.xml + document.xml.rels) from a collab-joiner editor, and collab-attach-user.test.ts asserts the tracked-change user is configured when supplied and left unset otherwise. collab-attach.test.ts is reshaped to the repo's provider-stub idiom (inert on(), explicit emit, synced/isSynced fields) and adds already-synced and synced-only sync cases, so the suite exercises the sync contract rather than a single auto-emitted edge. Adds yjs, y-websocket, and ws dependencies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 63c71be commit cf9684a

9 files changed

Lines changed: 538 additions & 11 deletions

File tree

apps/mcp/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
},
2020
"dependencies": {
2121
"@modelcontextprotocol/sdk": "^1.26.0",
22+
"ws": "^8.18.3",
23+
"y-websocket": "catalog:",
24+
"yjs": "catalog:",
2225
"zod": "^4.3.6"
2326
},
2427
"devDependencies": {
@@ -27,6 +30,7 @@
2730
"superdoc": "workspace:*",
2831
"@types/bun": "catalog:",
2932
"@types/node": "catalog:",
33+
"@types/ws": "catalog:",
3034
"typescript": "catalog:"
3135
},
3236
"publishConfig": {
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { describe, it, expect } from 'bun:test';
2+
import { Doc as YDoc } from 'yjs';
3+
import { buildAttachEditor } from '../session-manager.js';
4+
5+
/**
6+
* Tracked-change authoring over a collab attach requires a configured user.
7+
* Without one, `forceTrackChanges` rejects the edit ("forceTrackChanges requires
8+
* a user to be configured on the editor instance") because the gate reads
9+
* `editor.options.user`, which is null on a bare attach.
10+
*
11+
* `buildAttachEditor` accepts an optional user and wires it into the headless
12+
* Editor config so suggested edits can be attributed to a reviewer.
13+
*/
14+
describe('collab attach user identity (tracked-change user wiring)', () => {
15+
// Scope: this asserts buildAttachEditor wires `user` into the Editor config —
16+
// the input the forceTrackChanges gate reads. The gate's own behavior (rejecting
17+
// tracked edits when no user is set) belongs to super-editor and is tested there.
18+
it('configures the tracked-change user on the attach editor when supplied', async () => {
19+
const ydoc = new YDoc({ gc: false });
20+
const user = { id: 'reviewer-1', name: 'Reviewer', email: 'reviewer@example.com' };
21+
22+
const editor = await buildAttachEditor(ydoc, 'test-room', user);
23+
24+
// This is the exact value the forceTrackChanges gate reads.
25+
expect(editor.options.user).toEqual(user);
26+
27+
editor.destroy();
28+
});
29+
30+
it('leaves the user unset when none is supplied (default preserved)', async () => {
31+
const ydoc = new YDoc({ gc: false });
32+
33+
const editor = await buildAttachEditor(ydoc, 'test-room');
34+
35+
// Editor default for `user` is null; the no-arg attach path must not invent one.
36+
expect(editor.options.user ?? null).toBeNull();
37+
38+
editor.destroy();
39+
});
40+
});
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
import { describe, it, expect, mock, afterEach } from 'bun:test';
2+
import { tmpdir } from 'node:os';
3+
import { join } from 'node:path';
4+
import { randomBytes } from 'node:crypto';
5+
import { readFile, unlink } from 'node:fs/promises';
6+
import { SessionManager } from '../session-manager.js';
7+
8+
/**
9+
* `openRoom` orchestration (provider sync, sync timeout, awareness presence) and
10+
* the `save()` room-guard run against a live Yjs WebSocket server in production,
11+
* but the orchestration itself is socket-independent. Following the repo's collab
12+
* test idiom (createProviderStub in Editor.replace-file.test.ts), we inject a stub
13+
* provider so the logic is exercised without a server. The real socket transport
14+
* is the only part left to the end-to-end check in the PR description.
15+
*/
16+
17+
type ProviderEvent = 'sync' | 'synced';
18+
type SyncHandler = (synced?: boolean) => void;
19+
20+
/**
21+
* Provider stub mirroring the repo's collab test idiom (`createProviderStub` in
22+
* `Editor.replace-file.test.ts`): inert `on()`/`off()` that only register/deregister,
23+
* an explicit `emit()` the test drives by hand, and `synced`/`isSynced` fields. This is
24+
* what lets the suite exercise the real sync contract — the prior stub auto-emitted
25+
* `sync(true)` from inside `on()` and exposed no `synced`/`isSynced`, so it could only
26+
* test the one event shape and silently passed regardless of how the wait was wired.
27+
*
28+
* `synced: true` seeds an already-synced provider (pooled/reused) so openRoom's
29+
* already-synced precheck resolves with no emit.
30+
*/
31+
function providerStub({ synced = false }: { synced?: boolean } = {}) {
32+
const listeners: Record<ProviderEvent, Set<SyncHandler>> = {
33+
sync: new Set(),
34+
synced: new Set(),
35+
};
36+
const setLocalStateField = mock((_field: string, _value: unknown) => {});
37+
const destroy = mock(() => {});
38+
39+
const provider = {
40+
awareness: {
41+
setLocalStateField,
42+
getStates: () => new Map(),
43+
on() {},
44+
off() {},
45+
},
46+
synced,
47+
isSynced: synced,
48+
on(event: ProviderEvent, handler: SyncHandler) {
49+
listeners[event]?.add(handler);
50+
},
51+
off(event: ProviderEvent, handler: SyncHandler) {
52+
listeners[event]?.delete(handler);
53+
},
54+
emit(event: ProviderEvent, value?: boolean) {
55+
for (const handler of listeners[event]) handler(value);
56+
},
57+
destroy,
58+
};
59+
60+
return provider;
61+
}
62+
63+
/** Flush microtasks so openRoom reaches its suspended sync await and wires listeners. */
64+
const tick = () => new Promise<void>((r) => setTimeout(r, 0));
65+
66+
describe('superdoc_attach openRoom orchestration (stubbed provider)', () => {
67+
const sm = new SessionManager();
68+
const opened: string[] = [];
69+
const tempFiles: string[] = [];
70+
71+
afterEach(async () => {
72+
for (const id of opened.splice(0)) await sm.close(id).catch(() => {});
73+
for (const f of tempFiles.splice(0)) await unlink(f).catch(() => {});
74+
});
75+
76+
type Stub = ReturnType<typeof providerStub>;
77+
type AttachUser = { id?: string; name?: string; email?: string };
78+
79+
/**
80+
* Kick off openRoom, let it reach its suspended sync await and wire the provider
81+
* listener, then drive the provider to synced. `drive` defaults to the `sync(true)`
82+
* edge; pass a custom driver to exercise the `synced` (no-arg) path.
83+
*/
84+
async function attach(
85+
documentId: string,
86+
stub: Stub,
87+
{ user, drive = (s: Stub) => s.emit('sync', true) }: { user?: AttachUser; drive?: (s: Stub) => void } = {},
88+
) {
89+
const p = sm.openRoom('ws://test/doc', documentId, undefined, user, {
90+
createProvider: () => stub as unknown as never,
91+
});
92+
await tick();
93+
drive(stub);
94+
return p;
95+
}
96+
97+
it('returns a registered room session once the provider syncs', async () => {
98+
const stub = providerStub();
99+
const session = await attach('room-sync', stub);
100+
opened.push(session.id);
101+
102+
expect(session.id).toMatch(/^room-/);
103+
expect(session.filePath).toBeNull();
104+
expect(sm.list().some((s) => s.id === session.id)).toBe(true);
105+
});
106+
107+
it('returns immediately when the provider is already synced before attach', async () => {
108+
// Pooled/reused providers can be synced when handed to openRoom — no `sync`/`synced`
109+
// re-emit follows. The bespoke wait would hang here until a spurious timeout; the
110+
// helper's already-synced precheck resolves with no event. No emit is driven.
111+
const stub = providerStub({ synced: true });
112+
const session = await sm.openRoom('ws://test/doc', 'room-presynced', undefined, undefined, {
113+
createProvider: () => stub as unknown as never,
114+
});
115+
opened.push(session.id);
116+
117+
expect(session.id).toMatch(/^room-/);
118+
expect(sm.list().some((s) => s.id === session.id)).toBe(true);
119+
expect(stub.destroy).not.toHaveBeenCalled();
120+
});
121+
122+
it('resolves when the provider emits a no-arg synced event without a sync edge', async () => {
123+
// Some providers emit only `synced` (no boolean), never `sync(boolean)`. The prior
124+
// `sync`-only wait never resolved for these — this case is what makes the suite catch
125+
// that regression. The helper listens to `synced` too.
126+
const stub = providerStub();
127+
const session = await attach('room-synced-only', stub, { drive: (s) => s.emit('synced') });
128+
opened.push(session.id);
129+
130+
expect(session.id).toMatch(/^room-/);
131+
expect(sm.list().some((s) => s.id === session.id)).toBe(true);
132+
});
133+
134+
it('broadcasts awareness presence when a user is supplied', async () => {
135+
const stub = providerStub();
136+
const user = { id: 'reviewer-1', name: 'Reviewer', email: 'reviewer@example.com' };
137+
const session = await attach('room-presence', stub, { user });
138+
opened.push(session.id);
139+
140+
expect(stub.awareness.setLocalStateField).toHaveBeenCalledWith('user', user);
141+
});
142+
143+
it('does not touch awareness when no user is supplied', async () => {
144+
const stub = providerStub();
145+
const session = await attach('room-nopresence', stub);
146+
opened.push(session.id);
147+
148+
expect(stub.awareness.setLocalStateField).not.toHaveBeenCalled();
149+
});
150+
151+
it('rejects and tears down the provider when initial sync times out', async () => {
152+
// Never synced, never emits — openRoom's own timeout fires.
153+
const stub = providerStub();
154+
await expect(
155+
sm.openRoom('ws://test/doc', 'room-timeout', undefined, undefined, {
156+
createProvider: () => stub as unknown as never,
157+
syncTimeoutMs: 10,
158+
}),
159+
).rejects.toThrow(/sync timeout/);
160+
161+
expect(stub.destroy).toHaveBeenCalled();
162+
});
163+
164+
it('refuses to save a room session without an explicit output path', async () => {
165+
const stub = providerStub();
166+
const session = await attach('room-save-guard', stub);
167+
opened.push(session.id);
168+
169+
await expect(sm.save(session.id)).rejects.toThrow(/without specifying an output path/);
170+
});
171+
172+
it('saves a room session to an explicit output path', async () => {
173+
const stub = providerStub();
174+
const session = await attach('room-save-ok', stub);
175+
opened.push(session.id);
176+
177+
const out = join(tmpdir(), `mcp-collab-${randomBytes(6).toString('hex')}.docx`);
178+
tempFiles.push(out);
179+
180+
const result = await sm.save(session.id, out);
181+
expect(result.path).toBe(out);
182+
expect(result.byteLength).toBeGreaterThan(0);
183+
184+
const bytes = await readFile(out);
185+
expect(bytes[0]).toBe(0x50); // 'P' — PK zip magic
186+
expect(bytes[1]).toBe(0x4b); // 'K'
187+
});
188+
});
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { describe, it, expect } from 'bun:test';
2+
import { Doc as YDoc } from 'yjs';
3+
import { Editor } from 'superdoc/super-editor';
4+
import { buildAttachEditor } from '../session-manager.js';
5+
6+
/**
7+
* Regression: `superdoc_save` over a collab attach reported
8+
* "Exported document data is not binary (got undefined)".
9+
*
10+
* Root cause: the attach editor was built with no docx source, so
11+
* `converter.convertedXml` carried none of the base OOXML parts that
12+
* `Editor.exportDocx` unguarded-derefs (docProps/custom.xml, word/styles.xml,
13+
* word/_rels/document.xml.rels). The deref threw; the swallowing catch in
14+
* exportDocx returned `undefined`.
15+
*
16+
* A collab-joiner editor must export a valid .docx even with an empty Yjs doc.
17+
*/
18+
describe('collab attach export (superdoc_save over a room)', () => {
19+
it('exports binary .docx bytes from a collab-joiner editor with no docx source', async () => {
20+
const ydoc = new YDoc({ gc: false });
21+
const editor = await buildAttachEditor(ydoc, 'test-room');
22+
23+
const exported = await editor.exportDocument();
24+
25+
// The pre-fix failure mode: exportDocx throws on the missing parts and the
26+
// catch swallows to undefined.
27+
expect(exported).toBeDefined();
28+
29+
const bytes = exported instanceof Uint8Array ? exported : new Uint8Array(await (exported as Blob).arrayBuffer());
30+
31+
expect(bytes.byteLength).toBeGreaterThan(0);
32+
// Valid .docx is a ZIP — "PK" local-file-header magic.
33+
expect(bytes[0]).toBe(0x50); // 'P'
34+
expect(bytes[1]).toBe(0x4b); // 'K'
35+
36+
// Round-trip: the exported bytes must re-open as a structurally valid docx
37+
// carrying the base OOXML parts that were previously missing.
38+
const [parts] = (await Editor.loadXmlData(Buffer.from(bytes), true))!;
39+
const names = new Set(parts.map((p: { name: string }) => p.name));
40+
expect(names.has('word/document.xml')).toBe(true);
41+
expect(names.has('word/styles.xml')).toBe(true);
42+
expect(names.has('word/_rels/document.xml.rels')).toBe(true);
43+
44+
editor.destroy();
45+
});
46+
});

apps/mcp/src/__tests__/protocol.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@ import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'
66
const BLANK_DOCX = resolve(import.meta.dir, '../../../../shared/common/data/blank.docx');
77
const SERVER_ENTRY = resolve(import.meta.dir, '../index.ts');
88

9-
// 3 lifecycle + 10 intent tools from the generated catalog
9+
// 4 lifecycle + 10 intent tools from the generated catalog
1010
const EXPECTED_TOOLS = [
1111
// Lifecycle
1212
'superdoc_open',
13+
'superdoc_attach',
1314
'superdoc_save',
1415
'superdoc_close',
1516
// Intent tools (from catalog.json)
@@ -77,8 +78,10 @@ describe('MCP protocol integration', () => {
7778
const { tools } = await client.listTools();
7879

7980
// Multi-action intent tools should have an "action" property with an enum
81+
// superdoc_attach, like superdoc_open, is a session-creating lifecycle tool — no action enum.
8082
const multiActionTools = tools.filter(
81-
(t) => !['superdoc_open', 'superdoc_save', 'superdoc_close', 'superdoc_search'].includes(t.name),
83+
(t) =>
84+
!['superdoc_open', 'superdoc_attach', 'superdoc_save', 'superdoc_close', 'superdoc_search'].includes(t.name),
8285
);
8386

8487
for (const tool of multiActionTools) {
@@ -93,8 +96,11 @@ describe('MCP protocol integration', () => {
9396
await ready;
9497
const { tools } = await client.listTools();
9598

96-
// All intent tools (not lifecycle open) should require session_id
97-
const intentTools = tools.filter((t) => !['superdoc_open', 'superdoc_save', 'superdoc_close'].includes(t.name));
99+
// All intent tools (not session-creating lifecycle tools) should require session_id.
100+
// superdoc_open and superdoc_attach both produce a session_id rather than consuming one.
101+
const intentTools = tools.filter(
102+
(t) => !['superdoc_open', 'superdoc_attach', 'superdoc_save', 'superdoc_close'].includes(t.name),
103+
);
98104

99105
for (const tool of intentTools) {
100106
const schema = tool.inputSchema as { properties?: Record<string, unknown>; required?: string[] };

0 commit comments

Comments
 (0)