From 3acd164a06085d60fb586d72b870b0d97a3292d3 Mon Sep 17 00:00:00 2001 From: Rafael Dantas Justo Date: Wed, 8 Jul 2026 11:08:01 -0300 Subject: [PATCH] Chore: Warn when updating a Spaces page with an active editor draft Publishing page content through the REST API updates only the published content, not the collaborative OT editor draft ("Edit version"). When a page already has an active draft, that draft goes stale and a subsequent edit-and-publish from the Spaces web editor can silently overwrite the API-published content. This is a Spaces backend limitation with no REST surface to update the draft, so the MCP cannot fix the data itself. As an interim mitigation, twspaces-update_page now detects the dangerous case and warns without blocking: when a write changes published content (content set, or isPublish present) and the page's draftVersion > 1, the tool result is prefixed with a draft-sync warning that points the user to "Revert to last published version" in the web editor to resync. The draftVersion is read from the request argument first, falling back to a best-effort get only when absent/<=1, so the common path adds no extra call. Pages with draftVersion <= 1 (no real draft yet; the editor self-heals on first edit) produce no warning. The update_page tool description also states the limitation. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/twspaces/pages.go | 66 +++++++++++++++++++++++++++++---- internal/twspaces/pages_test.go | 55 +++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 7 deletions(-) diff --git a/internal/twspaces/pages.go b/internal/twspaces/pages.go index 9603c646..f9a84cc1 100644 --- a/internal/twspaces/pages.go +++ b/internal/twspaces/pages.go @@ -325,6 +325,29 @@ func PageDuplicate(httpClient *http.Client) toolsets.ToolWrapper { } } +// insufficientDraftVersion mirrors the Spaces web app's +// INSUFFICIENT_DRAFT_VERSION_NUMBER. A page whose draftVersion is <= 1 has no +// real collaborative editor draft yet: the editor seeds one from the published +// content the first time someone opens it. A draftVersion greater than this +// means an active editor draft exists. +const insufficientDraftVersion = 1 + +// draftOverwriteWarning is surfaced when an API content/publish write targets a +// page that already has an active editor draft. Content written through the REST +// API updates only the published page, not the collaborative editor draft, so +// the next person who edits the page in the Spaces web app sees the older draft +// and can silently overwrite what was just published. This is a known Spaces +// limitation (the API has no way to update the editor draft), not a problem with +// the arguments supplied to this tool. +const draftOverwriteWarning = "⚠️ Draft-sync warning: this page already has an active editor draft " + + "(draftVersion=%d). This update changes the PUBLISHED content only — it does NOT update the " + + "collaborative editor draft (\"Edit version\"). The next time someone opens this page in the Spaces " + + "web editor they will see the older draft, and saving from there can overwrite the content you just " + + "published. To resync the draft, open the page in the Spaces web editor and choose \"Revert to last " + + "published version\" from the ⋯ (more options) menu BEFORE making any edits — this replaces the stale " + + "draft with the content you just published. Alternatively, make this change in the Spaces web app " + + "instead. This is a known Spaces limitation and is unrelated to the values you passed." + // PageUpdate updates an existing page. func PageUpdate(httpClient *http.Client) toolsets.ToolWrapper { return toolsets.ToolWrapper{ @@ -333,7 +356,9 @@ func PageUpdate(httpClient *http.Client) toolsets.ToolWrapper { Annotations: &mcp.ToolAnnotations{ Title: "Update Page", }, - Description: "Update page.", + Description: "Update page. Note: content and publish changes update the published page only, not the " + + "live collaborative editor draft; if the page has an active editor draft, re-publishing from the " + + "Spaces web editor can overwrite these changes (known Spaces limitation).", InputSchema: &jsonschema.Schema{ Type: "object", Properties: map[string]*jsonschema.Schema{ @@ -478,15 +503,42 @@ func PageUpdate(httpClient *http.Client) toolsets.ToolWrapper { req.ReaderInlineCommentsEnabled = &v } - page, err := client.Pages.Update(ctx, - int64(arguments.GetInt("spaceId", 0)), - int64(arguments.GetInt("pageId", 0)), - req, - ) + spaceID := int64(arguments.GetInt("spaceId", 0)) + pageID := int64(arguments.GetInt("pageId", 0)) + + // Detect the known draft-sync limitation before writing: if this write + // changes the published content and the page already has an active editor + // draft, the write will not be reflected in that draft. Content updates + // already require draftVersion, so the risk is usually readable straight + // from the argument; fall back to a best-effort lookup otherwise. + _, hasPublish := arguments["isPublish"] + var draftWarning string + if req.Content != nil || hasPublish { + draftVersion := int64(arguments.GetInt("draftVersion", 0)) + if draftVersion <= insufficientDraftVersion { + if existing, gErr := client.Pages.Get(ctx, spaceID, pageID); gErr == nil && + existing != nil && existing.Page.DraftVersion != nil { + draftVersion = *existing.Page.DraftVersion + } + } + if draftVersion > insufficientDraftVersion { + draftWarning = fmt.Sprintf(draftOverwriteWarning, draftVersion) + } + } + + page, err := client.Pages.Update(ctx, spaceID, pageID, req) if err != nil { return nil, fmt.Errorf("failed to update page: %w", err) } - return helpers.NewToolResultJSON(page) + + result, err := helpers.NewToolResultJSON(page) + if err != nil { + return nil, err + } + if draftWarning != "" { + result.Content = append([]mcp.Content{&mcp.TextContent{Text: draftWarning}}, result.Content...) + } + return result, nil }, } } diff --git a/internal/twspaces/pages_test.go b/internal/twspaces/pages_test.go index 307c75f0..98ae6bbc 100644 --- a/internal/twspaces/pages_test.go +++ b/internal/twspaces/pages_test.go @@ -3,8 +3,10 @@ package twspaces_test import ( "net/http" + "strings" "testing" + "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/teamwork/mcp/internal/testutil" "github.com/teamwork/mcp/internal/twspaces" ) @@ -85,6 +87,59 @@ func TestPageUpdate(t *testing.T) { }) } +// TestPageUpdateWarnsWhenActiveDraft verifies that updating a page that already +// has an active editor draft (draftVersion > 1) surfaces the draft-sync warning, +// since the API write updates only the published content and not the draft. +func TestPageUpdateWarnsWhenActiveDraft(t *testing.T) { + mcpServer, cleanup := mcpServerMock(t, http.StatusOK, []byte(`{"page":{"id":10,"title":"Updated Page","slug":"getting-started","content":"

Updated

","state":"active","draftVersion":50,"space":{"id":1,"type":"space"}}}`)) + defer cleanup() + + testutil.ExecuteToolRequest(t, mcpServer, twspaces.MethodPageUpdate.String(), map[string]any{ + "spaceId": float64(1), + "pageId": float64(10), + "content": "

Updated

", + }, testutil.ExecuteToolRequestWithCheckMessage(func(t *testing.T, result mcp.Result) { + assertDraftWarning(t, result, true) + })) +} + +// TestPageUpdateNoWarnWhenNoActiveDraft verifies that updating a page whose +// draftVersion is <= 1 (no real editor draft yet) does not surface the warning, +// since the editor will seed the draft from the published content on first edit. +func TestPageUpdateNoWarnWhenNoActiveDraft(t *testing.T) { + mcpServer, cleanup := mcpServerMock(t, http.StatusOK, []byte(`{"page":{"id":10,"title":"Updated Page","slug":"getting-started","content":"

Updated

","state":"active","draftVersion":1,"space":{"id":1,"type":"space"}}}`)) + defer cleanup() + + testutil.ExecuteToolRequest(t, mcpServer, twspaces.MethodPageUpdate.String(), map[string]any{ + "spaceId": float64(1), + "pageId": float64(10), + "content": "

Updated

", + }, testutil.ExecuteToolRequestWithCheckMessage(func(t *testing.T, result mcp.Result) { + assertDraftWarning(t, result, false) + })) +} + +func assertDraftWarning(t *testing.T, result mcp.Result, want bool) { + t.Helper() + toolResult, ok := result.(*mcp.CallToolResult) + if !ok { + t.Fatalf("unexpected result type: %T", result) + } + if toolResult.IsError { + t.Fatalf("tool returned an error result: %v", toolResult.Content) + } + var found bool + for _, content := range toolResult.Content { + if textContent, ok := content.(*mcp.TextContent); ok && + strings.Contains(textContent.Text, "Draft-sync warning") { + found = true + } + } + if found != want { + t.Errorf("draft-sync warning present = %v, want %v", found, want) + } +} + func TestPageDelete(t *testing.T) { mcpServer, cleanup := mcpServerMock(t, http.StatusNoContent, []byte(``)) defer cleanup()