Sync ako/mxcli: pluggable widget action variables, ALTER pluggable property kinds, Starlark API docs

[ako]

Syncs ako/mxcli:main into mendixlabs/mxcli:main: 3 commits since #1204, landed in the fork through PRs ako#699–#701.

Pluggable widgets

• Carry an action's <actionVariables> from the .mpk (Pluggable widgets built from the .mpk lose their <actionVariables>: the action's ValueType is written with an empty ActionVariables list, and mx check reports CE0463 #1200). Widgets built from their .mpk wrote every action property with an empty ActionVariables list, so mx check reported CE0463 on every page carrying them, even with no action configured (Signature 2.1.0, Calendar 2.6.0 on 11.12.2). The parser now reads them, including nested object-list properties, and the generator writes CustomWidgets$WidgetActionVariable entries. Reconcile rewrites an embedded template's list only when it disagrees with the installed package, so an agreeing template keeps its entries and $IDs.
• ALTER PAGE writes a pluggable property to the field its kind declares (ALTER PAGE … SET ImageUrl = … ON <image> reports "Altered page" and changes nothing: the pluggable setter writes PrimitiveValue into a TextTemplate-kind property, so DESCRIBE, mx check and the runtime never see it #1201). SET ImageUrl = … ON img1 printed "Altered page" and changed nothing: every value went to PrimitiveValue, while the Image's imageUrl is a TextTemplate. The setter now dispatches on the declared type (TextTemplate, Expression, primitive). Kinds a plain value cannot express, a list value and a null text template are refused instead of reported as success.

Lint

• Every Starlark builtin and struct field documented in the write-lint-rules skill, with a test that holds the skill to the API registered in code. Seven query functions, two helpers and several struct fields were missing. Follow-up to Starlark rule API: documented literals drift from what the linter emits — microflow_type has three spellings, two of which never occur #1178

[github-actions]

AI Code Review

Review Summary

This PR syncs three commits from ako/mxcli:main addressing:

1. Pluggable widget action variables handling (Pluggable widgets built from the .mpk lose their <actionVariables>: the action's ValueType is written with an empty ActionVariables list, and mx check reports CE0463 #1200)
2. Pluggable property kind-aware ALTER PAGE SET (ALTER PAGE … SET ImageUrl = … ON <image> reports "Altered page" and changes nothing: the pluggable setter writes PrimitiveValue into a TextTemplate-kind property, so DESCRIBE, mx check and the runtime never see it #1201)
3. Starlark lint skill documentation completeness

What Looks Good

• Thorough bug fixes: Both Pluggable widgets built from the .mpk lose their <actionVariables>: the action's ValueType is written with an empty ActionVariables list, and mx check reports CE0463 #1200 and ALTER PAGE … SET ImageUrl = … ON <image> reports "Altered page" and changes nothing: the pluggable setter writes PrimitiveValue into a TextTemplate-kind property, so DESCRIBE, mx check and the runtime never see it #1201 address verified CE0463 issues with clear root causes and solutions
• Comprehensive testing:
  • New bug test MDL scripts (mdl-examples/bug-tests/1200-*, 1201-*)
  • Unit tests for MPK action variables parsing (modelsdk/widgets/mpk/actionvariables_test.go)
  • Widget augmentation/reconciliation logic (modelsdk/widgets/actionvariables_test.go)
  • Pluggable property kind handling (mdl/backend/pagemutator/pluggable_property_kind_test.go)
  • Lint skill coverage validation (mdl/linter/starlark_skill_coverage_test.go)
• Correct implementation:
  • Action variables now properly read from MPK XML and written to BSON as CustomWidgets$WidgetActionVariable entries
  • ALTER PAGE SET now dispatches to correct field (TextTemplate/Expression/PrimitiveValue) based on declared property kind
  • Lint skill now documents all Starlark builtins and struct fields with automated coverage test
• Maintains idempotency: Reconciliation only updates ActionVariables when they disagree with installed package
• Proper error handling: Refuses to set values for incompatible kinds (null templates, lists, structured types) instead of false success

Minor Issues

• Scope cohesion: While both widget fixes are related, the lint documentation update is a separate concern. However:
  • It's a small follow-up to Starlark rule API: documented literals drift from what the linter emits — microflow_type has three spellings, two of which never occur #1178
  • The coverage test provides immediate value
  • As a sync PR, bundling is acceptable
• Test placement: The pluggable_property_kind_test.go might benefit from being near other pagemutator tests, but current location is functional

Recommendation

Approve. The changes are:

• Correctly scoped to fix specific reported issues
• Fully tested with both unit and integration verification
• Consistent with existing code patterns and architecture
• Accompanied by validating MDL example scripts
• The lint update prevents future documentation/code drift

No blocking issues found. The PR successfully resolves CE0463 for Signature/Calendar widgets and fixes broken ALTER PAGE SET for pluggable properties while improving lint rule author experience.

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

[github-actions]

AI Code Review

Let me analyze this pull request based on the provided context and checklist.

The PR is titled "Sync ako/mxcli: pluggable widget action variables, ALTER pluggable property kinds, Starlark API docs" and includes:

1. Pluggable widgets fixes:
   • Carrying action's <actionVariables> from .mpk (fixes CE0463)
   • ALTER PAGE writing pluggable property to correct field based on declared type
2. Lint documentation improvements:
   • Documenting every Starlark builtin and struct field in write-lint-rules skill

Let me review against the checklist:

Overlap & duplication

• The PR includes a proposal "PROPOSAL_mdl_beta_syntax_freeze.md" which appears to be new documentation about MDL language critique and beta syntax freeze
• There are also new ADRs (0010, 0011, 0012) being added
• I need to check if these duplicate existing proposals or documentation
• Looking at the proposals list in CLAUDE.md, it says "Proposals in repo: No proposals directory." so this seems to be new content
• The ADRs being added (0010, 0011, 0012) appear to be new decisions being recorded

MDL syntax design

• The PR doesn't appear to add or modify MDL syntax directly (no changes to grammar files shown in the diff)
• The changes are mostly about fixing existing functionality and adding documentation
• The proposal document discusses syntax rules but doesn't appear to be implementing new syntax

Full-stack consistency

• Since no new MDL syntax is being added, this doesn't apply
• The fixes are to existing functionality (pluggable widget handling)

Test coverage

• The PR includes several new test files:
  • mdl/linter/starlark_skill_coverage_test.go (327 lines)
  • modelsdk/widgets/actionvariables_test.go (187 lines)
  • modelsdk/widgets/mpk/actionvariables_test.go (103 lines)
  • mdl/backend/pagemutator/pluggable_property_kind_test.go (169 lines)
  • mdl-examples/bug-tests/1200-mpk-action-variables.mdl (24 lines)
  • mdl-examples/bug-tests/1201-alter-page-set-pluggable-texttemplate.mdl (24 lines)
• This looks like good test coverage for the fixes being implemented

Security & robustness

• No obvious security issues spotted in the diff
• The changes seem to be about correctly handling data types and avoiding silent failures

Scope & atomicity

• The PR has a clear scope: fixing pluggable widget issues and documenting Starlark API
• It includes multiple related fixes that all pertain to the same general area (widget handling and lint documentation)
• The commits appear to be grouped logically:
  -docs(lint): document every Starlark builtin and struct field; test the whole surface ako/mxcli#699: Pluggable widgets - carry action's from .mpk
  -fix(alter): write a pluggable property to the field its kind declares (#1201) ako/mxcli#700: ALTER PAGE writes pluggable property to correct field
  -fix(widgets): carry an action's <actionVariables> from the .mpk (#1200) ako/mxcli#701: Lint - document Starlark builtins and struct fields
• This seems appropriately scoped

Code quality

• The changes look consistent with existing patterns
• No obvious old patterns being left behind
• The design docs being added (the proposal and ADRs) seem to match the implementation approach

Bugs & correctness

• The PR addresses two specific bugs:
  • Pluggable widgets built from the .mpk lose their <actionVariables>: the action's ValueType is written with an empty ActionVariables list, and mx check reports CE0463 #1200: Widgets losing action variables causing CE0463
  • ALTER PAGE … SET ImageUrl = … ON <image> reports "Altered page" and changes nothing: the pluggable setter writes PrimitiveValue into a TextTemplate-kind property, so DESCRIBE, mx check and the runtime never see it #1201: ALTER PAGE not writing to correct field for pluggable properties
• The fixes appear to be correct based on the descriptions:
  • For Pluggable widgets built from the .mpk lose their <actionVariables>: the action's ValueType is written with an empty ActionVariables list, and mx check reports CE0463 #1200: Reading action variables from .mpk and writing them correctly
  • For ALTER PAGE … SET ImageUrl = … ON <image> reports "Altered page" and changes nothing: the pluggable setter writes PrimitiveValue into a TextTemplate-kind property, so DESCRIBE, mx check and the runtime never see it #1201: Dispatching on declared type (TextTemplate, Expression, primitive) instead of always writing to PrimitiveValue

Let me look more closely at the specific changes:

For the pluggable widget action variables (#1200):

• Added ActionVariables field to PropertyDef struct in modelsdk/widgets/mpk/mpk.go
• Updated XML parsing to handle actionVariables
• Added buildActionVariablesArray and actionVariablesMatch functions
• Modified reconcileValueTypesFromMPK to use the action variables from the MPK
• Updated createDefaultValueType to use buildActionVariablesArray
• Added comprehensive tests

For the ALTER PAGE pluggable property kinds (#1201):

• Added buildPropKindMap function to determine property kind from widget schema
• Modified setPluggableWidgetPropertyMut to:
  • Check for list values and refuse them
  • Determine the correct field based on property kind (TextTemplate, Expression, PrimitiveValue)
  • Handle null TextTemplate case (refuse rather than write to hidden slot)
  • Handle Expression kind correctly
  • Refuse structured kinds (Action, DataSource, etc.) that can't be set with plain values
  • Keep primitive kinds in PrimitiveValue
• Added comprehensive tests

For the Starlark API documentation:

• Significantly expanded .claude/skills/mendix/write-lint-rules/SKILL.md to document:
  • New query functions: queues(), java_actions(), database_connections(), documents(), documentable_elements(), navigation_targets()
  • Struct fields for scheduled_event, password_policy, project_security
  • Helper functions like get_option
• Added test file mdl/linter/starlark_skill_coverage_test.go to verify the skill matches the actual API

Additionally, the PR includes:

• A large proposal document PROPOSAL_mdl_beta_syntax_freeze.md discussing MDL language critique and beta syntax freeze
• Three new ADRs (0010, 0011, 0012) documenting decisions about:
  • 0010: MDL canonical syntax rules
  • 0011: MDL language versioning
  • 0012: MDL-first and data-first editing
• Updates to existing documents:
  • .claude/skills/design-mdl-syntax.md (minor changes)
  • docs/13-decisions/0003-mdl-is-sql-shaped.md (minor change)
  • docs/13-decisions/README.md (added entries for new ADRs)

Let me check if there are any issues with the proposal and ADRs:

The proposal PROPOSAL_mdl_beta_syntax_freeze.md discusses MDL language critique and beta syntax freeze. It mentions decisions ADR-0010, ADR-0011, ADR-0012 which are being added in this PR. This seems consistent - the proposal is proposing these decisions, and the ADRs are recording them as accepted.

Looking at ADR-0010 (0010-mdl-canonical-syntax-rules.md):

• It establishes twelve syntax rules for MDL
• It's marked as Accepted
• Related to ADR-0003 and the proposal

ADR-0011 (0011-mdl-language-versioning.md):

• About MDL evolving through deprecation aliases and language header
• Accepted
• Related to ADR-0010 and the proposal

ADR-0012 (0012-mdl-first-and-data-first-editing.md):

• About MDL-first and data-first editing sharing syntax
• Accepted
• Builds on ADR-0008, ADR-0010, ADR-0011 and the proposal
• Related to PR docs: MDL beta syntax proposal and ADR-0010..0012 ako/mxcli#702

These seem to be legitimate architecture decisions being recorded.

Now, let me check against the specific review checklist items from CLAUDE.md:

Working Rules for a Change

Since this isn't a bug fix (it's a feature fix and documentation), I'll look at the general requirements:

• Fix-issue skill consulted - Not applicable as this isn't fixing a known bug pattern from the wiki
• Finding recorded - Not applicable for same reason
• Test written first - The PR includes tests, but I can't verify if they were written first
• Verified at the layer the symptom lives in - The fixes appear to be at the correct layer:
  • For Pluggable widgets built from the .mpk lose their <actionVariables>: the action's ValueType is written with an empty ActionVariables list, and mx check reports CE0463 #1200: Widget action variables - fixes in modelsdk/widgets/ (parsing MPK) and modelsdk/widgets/mpk/ (MPK parsing) and modelsdk/widgets/augment.go (reconcile)
  • For ALTER PAGE … SET ImageUrl = … ON <image> reports "Altered page" and changes nothing: the pluggable setter writes PrimitiveValue into a TextTemplate-kind property, so DESCRIBE, mx check and the runtime never see it #1201: ALTER PAGE pluggable properties - fixes in mdl/backend/pagemutator/ (where page mutations happen)
• Fix proven to be the cause - Can't verify from diff alone, but the tests demonstrate the fix works

Scope & atomicity

• Each commit does one thing - Based on the PR description, it's three commits:
  1. Pluggable widgets - carry action's from .mpk
  2. ALTER PAGE writes pluggable property to correct field
  3. Lint - document Starlark builtins and struct fields
     This looks correct - each commit addresses one concern
• Each PR scoped to single feature/concern - The PR description says it's syncing three commits from a fork, all related to improving widget handling and lint documentation. This seems reasonably scoped.
• Independent features in separate PRs - The three commits are related (all about improving correctness of widget handling and documentation), so keeping them together makes sense
• Refactors touching many files as own commit - No major refactors apparent

Code quality

• Refactors applied consistently - No obvious inconsistencies
• Manually maintained lists flagged as maintenance risks - The Starlark skill documentation is manually maintained, but the new test helps ensure it stays correct
• Design docs match implementation - The proposal and ADRs being added seem to describe the current/future state of the project

MDL syntax design

Since no new MDL syntax is being added, this section doesn't apply significantly. However, the proposal document discusses syntax rules, so let me check if it violates any:

• The proposal is documentation, not implementation
• It's discussing existing and proposed syntax, not adding new syntax
• It follows the guidelines by reading as English, using standard CRUD verbs, etc.

Full-stack consistency

Since no new MDL syntax is being added, this doesn't apply. The fixes are to existing functionality.

Let me check if there are any red flags:

1. The proposal document is very large (1362 lines) - but it's documentation, not code
2. The ADRs are being added - but they're documenting decisions that appear to have been made
3. The Starlark skill documentation is greatly expanded - but it's being verified by the new test

One thing to check: in the ALTER PAGE pluggable property fix, there's this comment:

// A null template is how #574 stores one its condition hides. ALTER
// does not re-run visibility, so building one here would put text in
// a pruned slot — refused, not created.

This references issue #574, which seems to be about conditional visibility. The fix correctly refuses to write to a null/hidden template rather than creating it in a hidden slot, which would be incorrect.

Another thing: in the same function, there's:

// Structured kinds cannot be expressed as a plain SET value. Before, they took
// a string in PrimitiveValue and reported success.

And then it refuses kinds like Action, DataSource, etc. This seems correct - you shouldn't be able to set complex properties with simple values.

For the action variables fix, the comment explains:

// The package's action variables. A template from an older
// widget version (or one cloned without them) otherwise keeps
// a list that disagrees with the installed definition — CE0463
// (#1200). Rewritten only when it differs, so a template that
// already agrees keeps its entries as they are.

This shows they're being careful to only update when necessary to preserve $IDs, which aligns with the GUID preservation principles in CLAUDE.md.

Let me check if the PR follows the "Definition of Done" from CONTRIBUTING.md (referenced in CLAUDE.md):

1. File an issue first - The PR references issues Pluggable widgets built from the .mpk lose their <actionVariables>: the action's ValueType is written with an empty ActionVariables list, and mx check reports CE0463 #1200 and ALTER PAGE … SET ImageUrl = … ON <image> reports "Altered page" and changes nothing: the pluggable setter writes PrimitiveValue into a TextTemplate-kind property, so DESCRIBE, mx check and the runtime never see it #1201
2. Get approval - Not visible in diff, but assumed done
3. Create feature branch - This is a sync from a fork
4. Use contributor commands - Not visible in diff
5. Validate locally - The PR includes tests that would be run with make test
6. Open a PR - Done

The PR also mentions:

• mx check reported CE0463 on every page carrying them (for Pluggable widgets built from the .mpk lose their <actionVariables>: the action's ValueType is written with an empty ActionVariables list, and mx check reports CE0463 #1200)
• SET ImageUrl = … ON img1 printed "Altered page" and changed nothing (for ALTER PAGE … SET ImageUrl = … ON <image> reports "Altered page" and changes nothing: the pluggable setter writes PrimitiveValue into a TextTemplate-kind property, so DESCRIBE, mx check and the runtime never see it #1201)

These are concrete symptoms that were fixed.

Let me check if there's anything that violates the MDL syntax design guidelines mentioned in CLAUDE.md:

• Reads as English (target audience is citizen developers)
• Uses standard CRUD verbs (CREATE/ALTER/DROP/SHOW/DESCRIBE)
• Consistent property format (Key: value) for property definitions
• AS for name-to-name mappings (old AS new)
• Qualified names (Module.Element)
• No keyword overloading
• No symbolic syntax
• Colon is for property definitions (Key: value), AS is for name mappings/renaming

Since no new MDL syntax is being added, these don't directly apply to the code changes. The proposal document discusses these guidelines but doesn't violate them.

One potential issue: I should check if the proposal document or ADRs introduce any syntax that violates these guidelines, but they're documentation, not executable MDL.

Let me look at one specific part of the proposal that might be concerning:

From PROPOSAL_mdl_beta_syntax_freeze.md:

### R1. One idempotent create, with one defined meaning

This is discussing a syntax rule, not implementing it. The actual implementation would be in the code, but I don't see changes to the grammar or visitor that implement this rule in this PR. The PR is adding documentation about the rule, not implementing the rule itself.

Actually, looking more carefully, the PR is adding the ADRs that record decisions about these rules, but the actual implementation of the rules would be in other PRs. This PR is just documenting the decisions.

This seems appropriate - first document and decide on the changes, then implement them in follow-up work.

Let me check if there are any missing pieces:

For the pluggable widget action variables fix:

• Does it handle nested object-list properties as mentioned in the PR description?
  • Looking at the mpk/actionvariables_test.go, there's a test for nested action variables in object properties
  • The test parses widget XML with nested properties and checks action variables
  • So yes, it appears to handle nested cases

For the ALTER PAGE pluggable property kinds fix:

• Does it handle all the cases mentioned?
  • TextTemplate: writes to TextTemplate field
  • Expression: writes to Expression field
  • Primitive: writes to PrimitiveValue
  • Null TextTemplate: refuses
  • List value: refuses
  • Structured kinds (Action, DataSource, etc.): refuses
  • This matches the PR description

For the Starlark API documentation:

• Does it document everything mentioned?
  • Seven query functions: queues(), java_actions(), database_connections(), documents(), documentable_elements(), navigation_targets(), and one more? (The PR description says seven)
  • Two helpers: gets mentioned get_option
  • Several struct fields: scheduled_event schedule fields, password_policy, project_security fields
  • The expanded skill document shows all these

One thing I noticed: in the Starlark skill coverage test, there's this line:

if len(names) < 10 {
    t.Fatalf("buildPredeclared returned %d names -- the enumeration is broken", len(names))
}

This suggests they expect at least 10 builtins, which seems reasonable.

Let me check if there are any testability concerns:

• The tests look comprehensive
• They use table-driven approaches where appropriate
• They check both positive and negative cases
• They verify the actual BSON structure being written

One potential improvement: in the pagemutator/pluggable_property_kind_test.go, there are tests for refusing structured kinds, but I don't see a test for what happens when you try to set a list value. Wait, actually there is:

// A bracketed value arrives as []string and %v fused its tokens into one
// string written as success (#750). No pluggable property takes a list.
func TestSetPluggableProperty_ListValueIsRefused(t *testing.T) {

So that's covered.

Let me check if the changes align with the architecture described in CLAUDE.md:

From CLAUDE.md:

• modelsdk/ is the MPR engine
• mdl/ is the language: grammar -> visitor -> ast -> executor
• api/ is the fluent builder layer
• cmd/mxcli/ is the CLI

The fixes are in:

• modelsdk/widgets/ - correct, this is part of the MPR engine for widgets
• mdl/backend/pagemutator/ - correct, this is where page mutations happen in the backend
• modelsdk/widgets/mpk/ - correct, MPK parsing is part of widget handling in modelsdk

The documentation changes are in:

• .claude/skills/ - correct, this is where skills live
• docs/11-proposals/ - correct, for proposals
• docs/13-decisions/ - correct, for ADRs

Everything is in the right place.

Let me check for any violations of the "Critical" concepts from CLAUDE.md:

GUID preservation:

• The action variables fix talks about keeping entries as they are when they already agree, to preserve $IDs
• This aligns with: "Preserve the stored GUID when rewriting an existing element."
• The comment says: "Reconcile rewrites an embedded template's list only when it disagrees with the installed package, so an agreeing template keeps its entries and $IDs."

ID preservation:

• From CLAUDE.md: "Writes Are Conditional, and an $ID Is Never

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source