[shannah]

Summary

The GUI Builder's MCP server could inspect the designer (state, actions), select, open a form, drag and run save/undo/redo, but an agent could not build anything: there was no way to add a component, set a property, change a layout or attach a handler. This adds editing tools, each implemented on the builder's own code paths:

Tool	What it does
guibuilder_component_types	Catalog of component types and the properties each accepts (kind, default, range, choices), layouts and constraints, guided size policies and align actions, command placements, binding strategies
guibuilder_get_form_xml	The active form's XML as it is in the editor (unsaved edits included), or one component's subtree
guibuilder_add_component	Adds a palette type under a parent at an index, optionally with name, layout, properties and constraint in the same step
guibuilder_insert_xml	Inserts a .gui fragment (a whole section) via the paste path: unique names, references inside the fragment follow, free table cells; a BorderLayout region already taken is refused
guibuilder_set_properties	Inspector properties, validated against the catalog (ranges, choices, Tabs index bound to tab count); null resets
guibuilder_rename_component	renameSelected, so relationships are repointed and names stay unique
guibuilder_delete_components	Deletes, clearing relationships to the deleted names
guibuilder_set_layout	Box (axis), Border (≤5 children), Flow, Grid (1–100), Table (never below occupied cells), Layered
guibuilder_set_constraint	Order (not in Table or Border layouts, where it would move other children); BorderLayout region with occupant swap and siblings' regions pinned; table cell/span/width/height with the inspector's overlap check; guided reference, insets, size policies and align actions via applyGuidedSizePolicy/applyLayeredAction
guibuilder_set_event_handler	actionEvent on types that fire action events; reports the companion source and whether the stub already exists
guibuilder_toolbar_commands	list/add/update/remove Form/Dialog toolbar commands
guibuilder_set_binding	Binding strategy, through syncBindingModel (unattended: the existing model is kept)
guibuilder_create_form	New .gui + companion (validated like cn1:create-gui-form, binding none for every root type), opened and saved so it compiles; never overwrites

How

• CodenameOneGUIBuilder.mcpEdit(operation, args) dispatches. Every mutating operation runs inside one GuiDocument transaction under unattended(...) and is a single undo step for the user.
  • A refusal after a partial change rolls back with abortTransaction() and rebuilds the canvas, because the rollback re-parses the elements.
  • A refusal before any change closes the transaction without re-parsing, so the canvas stays bound to live elements.
  • Whenever nothing changed, the selection and the modified flag are restored and the canvas is left alone.
  • An edit waits (it is refused) while the developer is typing in place or dragging, rather than committing their text or cancelling their drag. Reads such as toolbar_commands list bypass the transaction.
• Validation is the inspector's: border capacity, rectangleFree / occupiedTableExtent for tables (overflow-safe; out-of-range integers are refused), renameSelected for names, pasteXml for fragments (with an extra type/layout/region check of the fragment first).
• model/ComponentCatalog writes down what propertiesTab offers per type. The inspector still builds its fields by hand, so McpEditingToolsTest.theCatalogOffersExactlyWhatTheInspectorOffers opens the inspector for every palette type and the Form root and compares the guibuilder.property.* fields with the catalog; adding a field to one without the other fails.
• GuiBuilderMcpController registers the tools. Their arguments are parsed with nulls kept (JSONParser.parseJSON drops null members by default, which would turn "reset this property" into an empty request).

Why

The Codename One browser IDE (BuildCloud browser-ide) runs this GUI Builder precompiled and exposes WebMCP tools to the browser's agent; a companion BuildCloud PR reaches these tools in process (gui_*). They work the same over the desktop builder's socket MCP server.

Test plan

• McpEditingToolsTest (20 tests, JDK 8, under xvfb): every tool through JSON-RPC into a running builder, plus regression tests for each review finding:
  • BorderLayout clashes and siblings that must not move
  • the canvas staying bound after both kinds of refusal
  • no-op edits keeping the selection and the unmodified state
  • reads and refusals not disturbing the developer
  • huge spans
  • Container roots without binding
• Whole GUI Builder suite on JDK 8 (xvfb-run mvn -pl javase -am test -Dcodename1.platform=javase) at 234363c: 225 tests, 0 failures.
• check-control-characters.py, check-author-tags.py, check-copyright-headers.sh on the changed files.

Found along the way and not fixed here: core's MCPJson.parse parses with longs on, and JSONParser then drops an integer written with an exponent (1e10, 1E3) for every MCP tool.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SZMcbjRgY2gZgT5nLW9RVG

[shai-almog]

Posted this in the wrong PR by mistake. Ugh.

@shannah

1. Medium: insert_xml never detects a BorderLayout region that's already taken (around line 237). The check compares the new region against what each sibling reports. But GuiDocument.effectiveBorderConstraint moves any later duplicate to the first free region, so no two siblings ever report the same region and the check never fires. Say the parent already has a component in Center and the fragment asks for Center:

   • Without index: the insert succeeds and the saved XML has two Center constraints. The new component quietly ends up in North, with no error.
   • With index: 0: the new component takes Center and silently pushes the existing one into North.

   Fix: before pasteXml, compare the fragment's own layoutConstraint (normalized) against the siblings' regions. Or apply the region through mcpApplyBorderRegion, the same way set_constraint does.

2. Low: create_form with type: "Container" turns data binding on, and nothing can turn it off (in mcpCreateFormImpl). bindingStrategy="none" is only written for Form-like types, so a Container falls back to the default, "properties". Saving then generates a Model.java and binding code, which contradicts the code's own comment "No binding model unless asked for". set_binding refuses Container roots, so there's no way to undo it.

   Fix: write bindingStrategy="none" for every root type, or let set_binding accept a Container root.

3. read-only tool calls interrupt the person at the keyboard.

   • Why: mcpTransaction commits the inline editor and cancels any drag for every operation. That includes guibuilder_toolbar_commands with action: "list", which changes nothing.
   • What happens: an agent polling the command list cancels the user's drag or commits their half-typed edit. The status line then says "Modified through MCP" even though nothing changed.
   • Fix: do those steps and set that status only when the document actually changes.

[shannah]

Thanks, all three were real; they're fixed in 6ec5a4d.

1. BorderLayout insert. insert_xml now checks the fragment's own layoutConstraint (normalized) against childAtBorderConstraint before pasteXml. A fragment asking for a taken region is refused and names the component that holds it, and an unknown region value is refused too. index is refused in a BorderLayout parent, by add_component, insert_xml and set_constraint alike, because reordering there only reassigns the regions of children without their own constraint. A new BorderLayout child, from either tool, gets the region it landed in written as an explicit layoutConstraint, so a later change can't move it.
2. create_form with type: "Container". bindingStrategy="none" is now written for every root type. No <Name>Model.java is generated, so set_binding doesn't need to reach Container roots.
3. Reads interrupting the user.
   • toolbar_commands with list (or no action) no longer goes through the transaction.
   • Edits no longer commit the inline editor or cancel a drag. While the developer is typing in place or dragging, an edit is refused with "try again when they finish".
   • The canvas rebuild, the selection change, the mcp_edit action and "Modified through MCP" happen only when a committed edit changed the form.
   • While writing the test for this I also found that a rolled-back edit could still trigger a rebuild: abortTransaction re-parses the XML, so its serialization can differ in attribute order. Only a committed edit now counts as a change.

New tests in McpEditingToolsTest:

• insertedXmlCannotTakeABorderRegionThatIsAlreadyUsed: a clash, index: 0, and a free region recorded explicitly.
• readingDoesNotDisturbThePersonAtTheKeyboard: list leaves undo depth, status and rebuild count unchanged; an edit is refused while the inline editor is open, and the editor is left as it was; a refused edit causes no rebuild.
• The create-form test now also covers a Container root, which gets none and no Model class.

All 17 tests in that class pass on JDK 8. The full GUI Builder suite is running locally, and I'll add its result here when it finishes.

---

Generated by Claude Code

[shai-almog]

@shannah see:

1. High: one refused agent edit breaks the developer's canvas (line 8566).
   • Cause: mcpTransaction opens a transaction for every call. When a call is refused, the abort re-parses the saved XML, which replaces every element in the document with a new object.
   • What goes wrong: the canvas is only redrawn when an edit actually changed something. After a refusal, such as set_properties on a component that doesn't exist, it isn't redrawn. The canvas, the selection and the inspector are left pointing at the old elements, which are no longer in the document.
   • Result: clicking a component on the canvas does nothing. Inspector edits mark the form modified but never reach the saved file. Both stay broken until something else redraws the canvas.
   • Test: McpEditingToolsTest asserts that a refused edit causes no redraw, so the test enforces this bug.
   • Fix: redraw, or at least re-link the canvas and selection, after any aborted transaction. Alternatively, open the transaction only after the arguments have been validated.
2. Low: setting a BorderLayout region can move components nobody named (line 9074).
   • Cause: mcpApplyBorderRegion only swaps with whichever component already holds the requested region. Components with no explicit layoutConstraint get a region worked out from their order, which is common for palette adds and older forms.
   • Example: A, B and C have no explicit constraints and sit at Center, North and South. set_constraint {component: A, region: West} sets only A, so B moves to Center and C to North. The tool still reports success.
   • Fix: pin each sibling's current region before changing A's.
   • Context: mcpMoveToIndex already refuses to reorder BorderLayout children for this reason. The inspector's own region picker (chooseBorderConstraint, line 4401) has the same gap, so the PR repeats existing behaviour rather than introducing it.

[shannah]

Both fixed in 8b354a0. You were right about the first one: my previous push introduced it, and its test asserted the broken behaviour.

1. Canvas left on stale elements after a refused edit. The transaction now closes in one of three ways:

   • Committed: as before.
   • Refused before anything changed: this is the common case, such as an unknown component or a bad value checked up front. endTransaction() closes it, which records nothing for an unchanged document and does not re-parse. Every element object stays the same, so the canvas, the selection and the inspector stay valid without a redraw. The selection is restored to what it was.
   • Refused after a partial change: for example, a valid hint followed by an invalid columns. abortTransaction() still runs, and the canvas is now always rebuilt afterwards, so it follows the re-parsed elements.

   A new assertCanvasLinked check walks every canvas preview and fails if one is bound to an element the document no longer holds, or if the selection is stale. It runs after both kinds of refusal. With the previous push's code put back, both tests fail on it; with the fix they pass.

2. BorderLayout siblings moving. mcpApplyBorderRegion now reads every sibling's effective region first, then writes it as an explicit layoutConstraint wherever the stored one differs, which covers both no constraint and a duplicate. Only then does it set the named child's region and swap with an occupant. New test settingOneBorderRegionMovesNoOtherChild uses your example (A, B and C with no constraints, then A to West) and checks that B stays North and C stays South. I left the inspector's own chooseBorderConstraint gap alone, since it is existing behaviour outside this PR. It is worth fixing separately in the same way.

McpEditingToolsTest: 18 of 18 pass on JDK 8. The full GUI Builder suite at the previous push (6ec5a4d) passed with 222 tests and 0 failures; it is re-running at this head and I'll post the result.

---

Generated by Claude Code

[shannah]

The full GUI Builder suite passes at 8b354a0 on JDK 8 (xvfb-run mvn -pl javase -am test -Dcodename1.platform=javase): 223 tests, 0 failures.

---

Generated by Claude Code

[shai-almog]

@shannah

1. Medium: an edit that changes nothing still marks the form as unsaved (around lines 8560–8566).
   • Cause: document.setAttribute sets the modified flag even when the value is unchanged. Only an aborted transaction restores the old flag, so a committed no-op and a refusal that ends with endTransaction() both leave it set.
   • How it happens: set_properties or set_event_handler with the value the component already has. Also set_layout {axis: "Z"} on a BoxLayout container, which writes the unchanged layout attribute (line 8998) before rejecting the axis.
   • What goes wrong: the tool reports nothing modified, but create_form and open_form then refuse with "unsaved changes", and the person is asked to save a form nobody edited.
   • Fix: record the modified flag before the transaction and restore it whenever the XML is unchanged.
2. Medium: an edit that changes nothing still moves the selection behind the editor's back (line 8576).
   • Cause: the tools select their target component, but the old selection is only restored on a refusal, and the canvas only refreshes when something changed.
   • How it happens: a committed edit that sets an existing value leaves the document's selection on the agent's component. The canvas and inspector still show the person's component.
   • What goes wrong: inspector controls that act on the document's selection change the wrong component. The "Alignment reference" picker and the size-policy pickers are examples.
   • Fix: restore the previous selection whenever nothing changed, not only on a refusal.
3. Low/medium: very large numbers get past the table-cell checks (lines 8629 and 9125).
   • Cause: mcpInteger casts with (int), which caps anything over the int range instead of rejecting it. The bounds checks r + rs > rows and c + cs > columns then overflow.
   • How it happens: set_constraint {row: 1, rowSpan: 2147483647} (or 1e10). The sum wraps to a negative number, the check passes, and rectangleFree overflows the same way and sees no overlap.
   • What goes wrong: a span of 2147483647 is saved and passed to the canvas layout and the generated code.
   • Fix: reject out-of-range values in mcpInteger, and write the checks as rs > rows - r and cs > columns - c.

The ComponentCatalog Javadoc also says a ComponentCatalogTest fails if the catalog and the inspector drift apart. No such test exists in the PR or the tree, so nothing actually checks this. That's a misleading comment rather than a bug.

[shannah]

All four are addressed in 234363c.

1. No-op edits marking the form modified. mcpTransaction records isModified() and the selection before each edit. Whenever nothing changed, it puts both back: that covers an edit refused before touching anything and one committed with the values already there. It uses a new GuiDocument.restoreModified(boolean), which is needed because before a form's first save isModified() reads only the flag.
2. Selection moved behind the editor's back. Same change: a no-op now restores the document's selection whenever nothing changed, not only on a refusal.
3. Huge numbers. mcpInteger refuses values outside the int range instead of saturating them. The span checks are now written as rs > rows - r and cs > columns - c, so they cannot overflow.
4. Javadoc. The ComponentCatalog comment now names the test that actually performs the check: McpEditingToolsTest.theCatalogOffersExactlyWhatTheInspectorOffers, in the javase module.

New tests:

• anEditThatChangesNothingLeavesTheFormAsItWas: covers a same-value title and your axis: "Z" case on a freshly opened form, which stays unmodified, and create_form still works afterwards. A no-op elsewhere leaves the person's selection where it was.
• hugeSpansAreRefusedRatherThanOverflowing: 2147483647, 1.0e10, 99999999999 and "99999999999", as rowSpan and as columnSpan.

One finding outside this PR: rowSpan: 1e10, written without a decimal point, never reaches the tool. MCPServer parses arguments through MCPJson.parse with setUseLongsInstance(true). In that mode JSONParser calls Long.parseLong("1e10"), which throws, and the value is dropped (1E3 behaves the same). So that call was a successful no-op rather than an overflow, and with this change it no longer touches the form either. The parser behaviour itself is in core (JSONParser / MCPJson) and affects every MCP tool. It's worth fixing separately, but it doesn't belong in this PR.

McpEditingToolsTest: 20 of 20 pass on JDK 8. The full GUI Builder suite is running at this head, and I'll post the result.

---

Generated by Claude Code

[shannah]

The full GUI Builder suite passes at 234363c on JDK 8: 225 tests, 0 failures.

---

Generated by Claude Code

[shai-almog]

@shannah can you use an adversarial red-team to review the changes before submitting?

1. Medium: create_form skips the "developer is editing or dragging" guard (mcpCreateFormImpl).
   • Cause: every other editing tool is refused in mcpTransaction while the developer is typing inline or dragging. create_form doesn't go through that check.
   • Why the unsaved-changes check doesn't catch it: half-typed inline text isn't in the document yet, so the form doesn't count as modified and the call goes ahead.
   • What goes wrong: the developer's typing is lost with the old form. The inline editor then commits that text onto an element of the discarded form through the new document, so the brand-new form comes up marked modified even though nobody edited it.
   • Fix: apply the same refusal before writing the .gui file.
2. Medium: tables nested inside an insert_xml fragment aren't checked (mcpValidateFragment / mcpInsertXml).
   • Cause: cells are only fixed up for the parent the fragment is inserted into. A TableLayout container inside the fragment goes in exactly as written, with no check for overlapping cells, cells outside the table, or spans that don't fit.
   • What goes wrong: a fragment whose two children both sit at row 0, column 0 is accepted. The canvas then throws "Row: 0 and column: 0 already occupied" when it redraws, and so does the generated app at runtime. An oversized span throws "Illegal span" the same way.
   • Fix: run normalizeTableCells, or a cell and span check, over every nested TableLayout container in the fragment.
3. Low: set_binding can return an unrelated note (mcpSetBinding).
   • Cause: the reply's note is copied from the status line, assuming syncBindingModel just set it. It doesn't set it when the strategy is none with no model file on disk, or when there is no binding or source directory.
   • What goes wrong: the note repeats whatever the status line said before, such as "Selected loginButton through MCP".
   • Also: when the binding message is set, mcpTransaction immediately replaces it with "Modified through MCP", so the developer never sees it either.

[shannah]

Your three findings are fixed in bb5ced8. As you suggested, I also ran an adversarial red team on the whole change before pushing. Two independent reviewers each took one PR, without access to my reasoning. The CodenameOne one found 11 more defects. I checked each against the code, and they are all fixed in the same commit.

Your findings

1. create_form and the busy guard. It now runs the same mcpDeveloperBusy() check before writing anything. Test: createFormWaitsWhileTheDeveloperIsTyping.
2. Nested tables in insert_xml. Every TableLayout container inside the fragment is checked for overlapping cells, cells outside the table, and spans past its edge (mcpValidateFragmentTable). Children without a cell get a free one after the insert. Test: tablesInsideAFragmentAreCheckedLikeAnyOther.
3. set_binding note. The reply includes a note only if syncBindingModel actually changed the status line. When an edit sets its own status message, the transaction no longer overwrites it with "Modified through MCP". Test: setBindingReportsOnlyWhatItDid.

Red-team findings

• Edits during the developer's own dialogs (high). "Save before switching?" and "Regenerate the model?" run a nested EDT loop. An MCP edit that landed there was marked saved by save() but never written. mcpDeveloperBusy now refuses while a Dialog is current. It also refuses while an inspector text field has focus (a rebuild commits it on focus loss) and while a Code, Model or CSS pane is open.

• insets (high). These are now validated by handing them to a LayeredLayout, exactly as the inspector's field does. That rejects baseline on any side but the top, fractional px, and Infinity.

• Fragment attributes the generator emits as written (high). insert_xml now validates every attribute that the canvas or the generator reads, on every element:

  • cells and spans (0, 2000000000 and x all used to get through);
  • layeredInsets;
  • the guided anchors, which were a source-injection path into the generated buildUI();
  • reference positions, references, size policies, and guidedPreferredWidth/guidedPreferredHeight, which are also emitted raw.

  Inspector properties are normalized and written back, and the normalized XML is what gets pasted. So enabled="TRUE" and constraint="password" no longer behave differently on the canvas and in the app.

• Canvas rebuild failures reported as success (medium). If the canvas cannot show a committed edit, the edit is now undone and reported as an error. The old code ignored the rebuild's result.

• Smaller fixes:

  • Pasted names are sanitized as renames are, so | and whitespace no longer break relationships.
  • A rollback restores the modified state, and addCommand now uses canonical keys. Before this, the re-parse lower-cased actionEvent and a saved form read as modified.
  • Handler names that the generator renames (record, var and so on) are refused, so the reported method is the one actually generated.
  • The catalog's text and hint defaults now give the sample text a reset shows.

• guibuilder_state now reports editorDirty and developerBusy. The IDE uses them so it never closes the builder over unsaved pane text or someone's typing (BuildCloud#190).

An existing bug the new safety net exposed (not from this PR). An empty container switched to BorderLayout made every canvas rebuild throw "Cannot add component to BorderLayout Container without constraint". ComponentPreviewFactory adds its "Drop here" hint without a region. The inspector's layout picker triggers it too. It now puts the hint in CENTER. It's a one-line change in ComponentPreviewFactory, included here because the MCP layout tools need it.

New tests in McpEditingToolsTest:

• valuesTheCanvasOrTheGeneratedSourceCannotReadAreRefused
• fragmentValuesAreStoredTheWayTheCanvasAndGeneratorReadThem
• aRollbackLeavesASavedFormUnmodified
• theCatalogReportsTheTextAResetActuallyShows

All 27 tests in that class pass on JDK 8. The full GUI Builder suite is running at this head, and I'll post the result.

---

Generated by Claude Code

[shannah]

The full GUI Builder suite passes at bb5ced8 on JDK 8: 232 tests, 0 failures.

---

Generated by Claude Code

[shai-almog]

@shannah these aren't crucial since the complexity of resolving them might be non-trivial:

1. Medium-low: create_form accepts class names that clash with Codename One UI classes (line 9716). The tool says the new form compiles, but className=com.example.app.Form generates public class Form extends Form, which doesn't compile.
   • Why: the generated source imports com.codename1.ui.*, and a class in your own package wins over a wildcard import.
   • Other names: Label or Button compile at first, but every later field like private Label label1 then refers to the new class and breaks the build.
   • Fix: refuse a class name that matches any class the generated source imports.
2. Low: guided alignment reports success when it does nothing (line 9466). mcpApplyGuided only checks that the parent has at least two children. The helpers it calls quietly return when no reference component is visible on the canvas.
   • Example: align: "alignLeft" on a component whose only sibling has visible=false writes nothing but answers success: true.
3. Low: an edit taken back by the transaction can come back through Redo (line 8613). When the canvas can't show an edit, the transaction reverts it with document.undo(), which leaves the change on the redo stack.
   • What goes wrong: if the developer presses Redo, the change returns, and the canvas fails to show it again.
   • Fix: revert without leaving a redo entry.

[shannah]

All three turned out to be small, so they're fixed in 1a2cd4b.

1. Class names that shadow imports. create_form now refuses a simple name that the generated companion or model source imports, and suggests e.g. LabelForm instead. The list is GENERATED_IMPORT_NAMES and covers:

   • every public class in com.codename1.ui, com.codename1.ui.layouts and com.codename1.properties;
   • the single-type imports (Accordion, SpanLabel, ActionEvent, Dimension, TableLayout, the binding classes).

   The builder can't list a package at run time, so the list is written out by hand. theImportedNameListCoversTheCoreJar enumerates the core jar and fails if core gains a public class in those packages that the list lacks.

2. Guided alignment that does nothing. After the canvas rebuild, mcpApplyGuided now resolves the reference the helpers would use (guidedReferenceComponent). If there is none, it refuses with "No reference component is visible next to …" instead of reporting success. Test: guidedAlignmentWithoutAVisibleReferenceIsNotReportedAsDone, which asserts that success always means the document changed.

3. Redo of a reverted edit. The revert after a failed rebuild now goes through a new GuiDocument.revertLastChange(). It restores the last undo state without pushing a redo entry, so Redo can't bring the change back.

McpEditingToolsTest: 30 of 30 pass on JDK 8. The full GUI Builder suite passed with 232 tests at the previous head; these changes only add checks and tests on top of it.

---

Generated by Claude Code