Skip to content

Commit b67a833

Browse files
akoclaude
andcommitted
feat(mcp): wire page-level SET Title over MCP (ALTER PAGE)
Page-level SET (no `on` clause) previously rejected every property with a generic "not yet supported". Wire SET Title onto the pg LightPage's top-level `title` string (the same key CreatePage writes and pg_read_page returns), so `alter page … set (Title = '…')` works over the MCP backend. Url and the pop-up settings (PopupWidth/Height/Resizable/CloseAction) are not part of the pg LightPage shape — its schema is additionalProperties:false, so injecting those keys would make pg_patch_page reject the whole root-replace patch (taking any bundled Title change with it). They are rejected at SET time, before Save(), with an accurate message, so they never poison the content tree; set them in Studio Pro. Also refresh PED_MCP_CAPABILITIES.md (pg_write_page → pg_patch_page in the ALTER PAGE paragraph, page-level SET Title documented) and capabilities.yaml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f941c98 commit b67a833

4 files changed

Lines changed: 84 additions & 25 deletions

File tree

docs/03-development/PED_MCP_CAPABILITIES.md

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -260,20 +260,26 @@ use, so it has its own success check (contains "success").
260260

261261
**ALTER PAGE** is a read-modify-write on the pg tree: `OpenPageForMutation` loads
262262
the page via `pg_read_page`, the mutator edits the in-memory tree, and `Save()`
263-
writes it back via `pg_write_page`. Supported in-place ops: INSERT (before/after a
264-
widget), DROP widget, REPLACE widget, SET DataSource, SET Layout, and **SET widget
265-
property** (`set (prop = value, …) on <widget>`) — plus the introspection the
266-
executor needs (FindWidget, WidgetScope, ParamScope, EnclosingEntity). The widget
267-
ref is the widget name (recursive tree search). The executor passes the AST
268-
position token (`"AFTER"`/`"BEFORE"`), so the mutator compares case-insensitively.
269-
SET maps the MDL property name (also case-insensitive) to its pg key: Class/Style
270-
→ the widget's `appearance`; Caption/Content/Label → the `ct:`-prefixed client
271-
templates; ButtonStyle → pg's normalized enum; TabIndex/RenderMode/Editable/Name →
272-
direct keys; Visible → a conditional-visibility expression. Not yet mapped: unknown
273-
SET properties, column INSERT/REPLACE/property, design properties,
274-
pluggable-property SET, and page variables — each returns a clear error.
275-
276-
Pages use a **separate protocol**: `pg_write_page` / `pg_read_page` (PED is
263+
writes it back via `pg_patch_page` (a root-replace patch). Supported in-place ops:
264+
INSERT (before/after a widget), DROP widget, REPLACE widget, SET DataSource, SET
265+
Layout, **SET widget property** (`set (prop = value, …) on <widget>`), and
266+
**page-level SET Title** (`set (Title = '…')` with no `on` clause) — plus the
267+
introspection the executor needs (FindWidget, WidgetScope, ParamScope,
268+
EnclosingEntity). The widget ref is the widget name (recursive tree search). The
269+
executor passes the AST position token (`"AFTER"`/`"BEFORE"`), so the mutator
270+
compares case-insensitively. SET maps the MDL property name (also
271+
case-insensitive) to its pg key: Class/Style → the widget's `appearance`;
272+
Caption/Content/Label → the `ct:`-prefixed client templates; ButtonStyle → pg's
273+
normalized enum; TabIndex/RenderMode/Editable/Name → direct keys; Visible → a
274+
conditional-visibility expression. Page-level SET Title maps onto the LightPage's
275+
top-level `title` string. Not yet mapped: page-level Url and pop-up settings
276+
(PopupWidth/Height/Resizable/CloseAction) — the LightPage schema is
277+
`additionalProperties:false`, so they aren't on the pg shape and are rejected
278+
before `Save()` (set them in Studio Pro); also unknown SET properties, column
279+
INSERT/REPLACE/property, design properties, pluggable-property SET, and page
280+
variables — each returns a clear error.
281+
282+
Pages use a **separate protocol**: `pg_patch_page` / `pg_read_page` (PED is
277283
forbidden for pages). The backend maps the executor's `pages.Page` (shell +
278284
LayoutCall slots + widget tree) onto the high-level page content. Note pg's
279285
container type is **`Pages$DivContainer`** (not `Container`); page reads

mdl/backend/mcp/capabilities.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ features:
4545
- key: pages
4646
feature: "Pages"
4747
available: true
48-
note: "CREATE + ALTER (widget coverage grows per type)"
48+
note: "CREATE + ALTER (widget coverage grows per type); ALTER page-level SET Title (maps onto the pg LightPage's top-level title). Url and pop-up settings (PopupWidth/Height/Resizable/CloseAction) are NOT on the pg LightPage shape (schema is additionalProperties:false) — set them in Studio Pro"
4949
- key: workflows
5050
feature: "Workflows"
5151
available: true

mdl/backend/mcp/page_mutator.go

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -339,11 +339,8 @@ func (m *mcpPageMutator) SetWidgetProperty(widgetRef, prop string, value any) er
339339
if widgetRef == "" {
340340
// Page-level SET (no ON clause): Title, Url, PopupWidth/PopupHeight/
341341
// PopupResizable, etc. These map onto the pg page object itself, not a
342-
// widget. Their pg content-tree representation hasn't been probed/mapped
343-
// against pg_read_page yet, so reject honestly rather than guess at the
344-
// key names (and avoid the misleading "widget \"\" not found"). The MPR
345-
// engines support these via the shared pagemutator's applyPageLevelSetMut.
346-
return fmt.Errorf("setting page-level property %q is not yet supported by the MCP backend", prop)
342+
// widget.
343+
return m.setPageLevelProperty(prop, value)
347344
}
348345
_, _, _, w, ok := findWidget(m.content, widgetRef)
349346
if !ok {
@@ -390,6 +387,34 @@ func (m *mcpPageMutator) SetWidgetProperty(widgetRef, prop string, value any) er
390387
return nil
391388
}
392389

390+
// setPageLevelProperty applies a page-level SET (no widget target) onto the pg
391+
// LightPage content tree. Pages go through the pg_* protocol, not PED, so the
392+
// only page-level fields that can be set are the ones the LightPage exposes.
393+
//
394+
// The LightPage carries the page title as a plain top-level "title" string (the
395+
// same key CreatePage writes and pg_read_page returns). Url and the pop-up
396+
// settings (PopupWidth/Height/Resizable/CloseAction) are NOT part of the
397+
// LightPage shape — its schema is additionalProperties:false, so injecting those
398+
// keys would make pg_patch_page reject the whole root-replace patch (taking any
399+
// bundled Title change down with it). They are rejected here, before Save(), so
400+
// they never poison the content tree; set them in Studio Pro. (If a future pg
401+
// server exposes them on the LightPage, add the mapping here.)
402+
func (m *mcpPageMutator) setPageLevelProperty(prop string, value any) error {
403+
switch strings.ToLower(prop) {
404+
case "title":
405+
s, ok := value.(string)
406+
if !ok {
407+
return fmt.Errorf("page Title must be a string, got %T", value)
408+
}
409+
m.content["title"] = s
410+
return nil
411+
default:
412+
return fmt.Errorf("setting page-level property %q is not supported by the MCP backend "+
413+
"(pages go through the pg_* tools, not PED; the pg LightPage exposes only the page Title at page level — "+
414+
"set Url and pop-up settings in Studio Pro)", prop)
415+
}
416+
}
417+
393418
func toStr(v any) string {
394419
if s, ok := v.(string); ok {
395420
return s

mdl/backend/mcp/page_mutator_test.go

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,35 @@ func TestPageMutator_SetLayoutAndUnsupported(t *testing.T) {
134134
}
135135
}
136136

137-
// Page-level SET (empty widgetRef) — Title / pop-up dimensions / etc. — is not
138-
// yet mapped onto the pg content tree. It must reject clearly rather than report
139-
// the misleading "widget \"\" not found" (issue #661).
140-
func TestPageMutator_PageLevelSet_Rejected(t *testing.T) {
141-
for _, prop := range []string{"Title", "PopupWidth", "PopupHeight", "PopupResizable"} {
137+
// Page-level SET Title (empty widgetRef) maps onto the pg LightPage's top-level
138+
// "title" key — the same key CreatePage writes and pg_read_page returns.
139+
func TestPageMutator_PageLevelSet_Title(t *testing.T) {
140+
m := newTestMutator()
141+
if err := m.SetWidgetProperty("", "Title", "Edit Expense"); err != nil {
142+
t.Fatalf("SET Title: %v", err)
143+
}
144+
if m.content["title"] != "Edit Expense" {
145+
t.Fatalf("title = %v, want \"Edit Expense\"", m.content["title"])
146+
}
147+
// Lowercase from the executor must work too.
148+
if err := m.SetWidgetProperty("", "title", "Lower"); err != nil {
149+
t.Fatalf("SET title (lowercase): %v", err)
150+
}
151+
if m.content["title"] != "Lower" {
152+
t.Fatalf("title = %v, want \"Lower\"", m.content["title"])
153+
}
154+
// A non-string Title is rejected (no silent coercion).
155+
if err := m.SetWidgetProperty("", "Title", 42); err == nil {
156+
t.Error("non-string Title should be rejected")
157+
}
158+
}
159+
160+
// Url and pop-up settings are not part of the pg LightPage shape (its schema is
161+
// additionalProperties:false), so they must reject clearly — before Save(), so
162+
// they never poison the content tree — rather than report the misleading
163+
// "widget \"\" not found" (issue #661).
164+
func TestPageMutator_PageLevelSet_Unsupported(t *testing.T) {
165+
for _, prop := range []string{"Url", "PopupWidth", "PopupHeight", "PopupResizable", "PopupCloseAction"} {
142166
m := newTestMutator()
143167
err := m.SetWidgetProperty("", prop, 800)
144168
if err == nil {
@@ -147,6 +171,10 @@ func TestPageMutator_PageLevelSet_Rejected(t *testing.T) {
147171
if got := err.Error(); !contains(got, "page-level property") || !contains(got, "MCP backend") {
148172
t.Errorf("%s: unclear error %q", prop, got)
149173
}
174+
// The unsupported key must NOT have leaked into the content tree.
175+
if _, leaked := m.content[strings.ToLower(prop)]; leaked {
176+
t.Errorf("%s: rejected property leaked into content tree", prop)
177+
}
150178
}
151179
}
152180

0 commit comments

Comments
 (0)