Sync ako/mxcli: typed retrieve constraints, security reference checks, model-level MDL-MAP04

[ako]

Syncs ako/mxcli:main into mendixlabs/mxcli:main: 4 commits since #1334, landed in the fork through PRs ako#1021, ako#1023 and ako#1027.

Check

• Retrieve constraints comparing an attribute with a variable of an incompatible type are reported by check --references (Retrieve XPath comparing an attribute with a variable of another type (enum vs String, String vs DateTime, Integer vs String) passes check; mx check CE0161 #1325). Examples are Kind = $Filter with $Filter a String, or Email >= $Since with a DateTime; these were CE0161 at build time. The compatibility table is measured on mxbuild 11.14.0 over 8 attribute × 8 variable types, and it is not plain type equality (a String variable against a DateTime attribute builds clean; Integer/Long/Decimal mix freely).
• GRANT / REVOKE and user-role statements resolve what they name. That covers entities, documents, members, module roles and user roles, against the project and what the script has created so far. All of these printed "Check passed!" before, and create user role / alter user role … add / create demo user wrote unknown roles for mxbuild to report as CE1613.

Lint

• MDL-MAP04 runs against the model (lint misses import-from-mapping activities stored as First on an object-rooted mapping (runtime: key not found: Path(QName(None,),None,)); MDL-MAP04 only checks scripts #1319). An import activity stored with First over an object-rooted mapping passed lint and mx check, then threw key not found: Path(QName(None,),None,) at runtime. check refused it only in a script, so activities written by v0.24.0 or with exec --no-check were found by nothing. lint now checks stored import-from-mapping and REST returns-mapping activities.
• Refactor: the import-mapping root-shape helpers move to mdl/types, so the lint rule, the builder and check share one decision

[github-actions]

AI Code Review

Review Summary

This PR syncs changes from ako/mxcli:main to mendixlabs/mxcli:main, implementing three key improvements:

1. Type validation for retrieve constraints (issue Retrieve XPath comparing an attribute with a variable of another type (enum vs String, String vs DateTime, Integer vs String) passes check; mx check CE0161 #1325)
2. Reference resolution for GRANT/REVOKE and user-role statements
3. Model-level lint rule for MDL-MAP04 (issue lint misses import-from-mapping activities stored as First on an object-rooted mapping (runtime: key not found: Path(QName(None,),None,)); MDL-MAP04 only checks scripts #1319)

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

None found.

What Looks Good

• Thorough implementation: Each feature properly wired through the full pipeline where applicable
• Comprehensive testing: Extensive test cases using real fixtures (PedApp) covering edge cases
• Code quality: Proper refactoring to eliminate duplication (mapping shape helpers moved to shared package)
• Documentation: Updated skill files, syntax help, and built-in rules documentation
• Atomic commits: Each logical change in its own commit
• Error handling: Clear, actionable error messages with helpful hints
• Integration: Proper connection to existing validation/lint pipelines

Recommendation

Approve - The PR successfully addresses the reported issues with high-quality implementations that follow project patterns and improve the tool's ability to catch problems early. All validation is properly integrated, tests are comprehensive, and documentation is updated appropriately. No critical issues identified.

---

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

[github-actions]

AI Code Review

Review Summary

This PR syncs changes from ako/mxcli:main to fix three specific issues:

1. Retrieve operand type validation (Retrieve XPath comparing an attribute with a variable of another type (enum vs String, String vs DateTime, Integer vs String) passes check; mx check CE0161 #1325) - adds check --references detection for type mismatches in retrieve constraints
2. Grant/REVOKE reference resolution - makes check --references validate security statement references against the project
3. MDL-MAP04 lint rule enhancement (lint misses import-from-mapping activities stored as First on an object-rooted mapping (runtime: key not found: Path(QName(None,),None,)); MDL-MAP04 only checks scripts #1319) - extends existing lint rule to detect problematic import activities already in the model

The changes are well-scoped, thoroughly tested, and follow the project's architectural patterns. No critical issues were found.

What Looks Good

• Comprehensive test coverage: New unit tests for all three fixes, integration tests using real PedApp fixture, and MDL example files demonstrating the bugs
• Proper full-stack wiring:
  • Grant reference validation: Added to validate.go (microflow/nanoflow paths) with corresponding executor handler
  • Retrieve operand validation: Added to validate.go with test coverage
  • MDL-MAP04 lint rule: Registered in cmd_lint.go, uses shared helpers from mdl/types, includes model-level checking
• Thoughtful refactor: Moving import-mapping root-shape helpers to mdl/types eliminates duplication between lint rule, builder, and check-time validation
• Clear documentation: Updated skills and CHANGELOG.md explain the changes
• Robust error handling: Validation functions properly handle disconnected states and edge cases

Minor Issues

1. Inconsistent error message formatting in validate_grant_refs.go:

   • Some error messages use past tense ("does not exist") while others use present tense ("is not found")
   • Suggestion: Standardize on present tense for consistency with other mxcli validation messages

2. Missing module existence check in grant reference validation:

   • While the validation checks role existence within a module, it doesn't explicitly validate that the module itself exists
   • However: The test cases show it does catch non-existent modules (e.g., "NopeMod.User" reports role not found), suggesting the implementation implicitly handles this by checking the module's role list

3. Slightly complex control flow in grantResolver.recordAutoDocumentRole:

   • The nested conditions for detecting when to auto-create <Module>.User role are correct but dense
   • Not critical: The logic aligns with platform behavior and is well-tested

Recommendation

Approve the PR. The changes:

• Fix real user-reported bugs with Mendix Studio Pro compatibility
• Follow the project's validation patterns (lazy loading, statement-order processing)
• Include appropriate test coverage at unit, integration, and example levels
• Maintain backward compatibility (no breaking changes to MDL syntax)
• Address the specific overlap concerns (no duplication with existing features)

The PR successfully implements the required full-stack validation for the fixed issues while maintaining the project's quality standards. The minor issues noted are documentation/refinement opportunities that don't block merging.

---

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

[github-actions]

AI Code Review

Review Summary

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

• File length in mdl/executor/validate_grant_refs.go: At 651 lines, this file is quite long and may impact maintainability. While the complexity is justified by the comprehensive reference validation logic, consider splitting it into smaller focused files (e.g., separate validators for entity grants, microflow grants, user roles) in a follow-up refactor.

What Looks Good

• Thorough bug fixes:
  • Retrieve operand type validation (Retrieve XPath comparing an attribute with a variable of another type (enum vs String, String vs DateTime, Integer vs String) passes check; mx check CE0161 #1325) now catches CE0161 at check time with clear error messages and helpful hints (especially for enumerations).
  • Security reference validation resolves GRANT/REVOKE/user-role statements against both project definitions and script-created elements, preventing CE1613 errors.
  • MDL-MAP04 lint rule now operates at model level (not just scripts) to catch runtime import mapping failures.
  • OData authentication refactor properly handles the transition from trailing clause to property-based syntax with correct deprecation handling and roundtrip integrity.
• Excellent test coverage:
  • New unit tests for all validations (validate_retrieve_operand_types_test.go, validate_grant_refs_test.go).
  • Integration test using PedApp fixture for grant references (check_grant_references_pedapp_test.go).
  • Comprehensive lint rule tests (import_range_object_mapping_test.go).
  • OData authentication tests covering visitor, executor, upgrade, and describe roundtrip scenarios.
• Full-stack consistency:
  • OData authentication changes properly span AST (ast_odata.go), visitor (visitor_odata.go), executor (cmd_odata.go), and tests.
  • Mapping shape logic centralized in mdl/types/mapping_shape.go shared by builder, check, and linter.
  • No missing pipeline components for modified features.
• Correct MDL syntax handling:
  • Property definitions correctly use colon (Authentication: ...).
  • AS keyword only used in appropriate mapping contexts (not present in this PR).
  • Statements read as English with standard CRUD verbs.
  • No keyword overloading or symbolic syntax.
• Atomic commits: Each commit addresses a single concern (retrieve types, grant references, lint rule, OData

---

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

[github-actions]

AI Code Review

Review Summary: This PR syncs changes from a fork implementing three main features: typed retrieve constraints (#1325), security reference checks (grant/revoke statements), and model-level MDL-MAP04 lint rule. It also includes unrelated OData authentication property improvements. The changes follow good practices with thorough testing and full-stack wiring where applicable.

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

---

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

[github-actions]

AI Code Review

Review Summary

This PR syncs changes from ako/mxcli:main to mendixlabs/mxcli:main, implementing three main features:

1. Typed retrieve constraints validation (issue Retrieve XPath comparing an attribute with a variable of another type (enum vs String, String vs DateTime, Integer vs String) passes check; mx check CE0161 #1325) - validates attribute/variable type compatibility in XPath constraints
2. Security reference resolution (grant/revoke statements) - resolves referenced entities/roles against project state
3. Model-level MDL-MAP04 lint rule (issue lint misses import-from-mapping activities stored as First on an object-rooted mapping (runtime: key not found: Path(QName(None,),None,)); MDL-MAP04 only checks scripts #1319) - detects import activities storing "First" over object-rooted mappings

The changes are well-scoped, thoroughly tested, and follow project conventions. No critical issues were found.

What Looks Good

• Complete validation coverage:
  • Retrieve operand types validation handles all type combinations per mxbuild 11.14.0 measurements
  • Grant/reference validation covers all security statement types (entity/microflow/nanoflow/page/OData/REST/service) and user/role statements
  • MDL-MAP04 lint rule correctly identifies the runtime failure scenario and provides helpful remediation
• Thorough testing:
  • New test files for each feature (validate_grant_refs.go, validate_retrieve_operand_types.go, import_range_object_mapping.go)
  • PedApp fixture testing for grant validation
  • Comprehensive test cases covering edge cases (inheritance, System module, auto-created roles)
  • Whitespace handling tests for Java action parameters (A microflow argument followed by a line break before the closing parenthesis is stored with the line break (CE1613 only in mx check) #1282)
• Proper architectural layering:
  • Validation logic placed in executor layer (consistent with other validations)
  • Lint rule shares mapping shape logic with builder via mdl/types/mapping_shape.go
  • OData authentication property refactor properly handles deprecation and backward compatibility
• Documentation updates:
  • CHANGELOG entries
  • Skill updates for JSON mappings, OData sharing, and lint rule writing
  • Quick reference and internals documentation updated
• Atomic commits: Each sync commit addresses a single concern (retrieve types, grant references, MDL-MAP04/refactor)

Minor Issues

1. In mdl/executor/validate_grant_refs.go:

   • Line 415: g.recordAutoDocumentRole(s.Name.Module) could be simplified to g.recordAutoDocumentRole(s.Name.String()) since QualifiedName.String() returns the module part when name is empty (though current usage is correct)
   • Line 528: Comment says "the statement's exec refuses an unknown role" but the function handles both grant and revoke - minor wording inconsistency

2. In mdl/executor/validate_retrieve_operand_types.go:

   • Line 45: The comment mentions String variable against DateTime attribute builds, but the compatibility table shows this is actually allowed (correctly implemented in code)
   • Line 147: xpathOperandTypeName could use a guard clause for enumeration to reduce nesting

3. In mdl/linter/rules/import_range_object_mapping.go:

   • Line 100: The violation message repeats the mapping name twice ("stores Studio Pro's First (%s), but %s is object-rooted") - slightly redundant but not incorrect
   • Line 128: Suggestion mentions $X = import from mapping ... ($Json); but should note that $Json needs to be defined elsewhere (though this is implied by context)

4. In documentation updates:

   • docs-site/src/internals/catalog-schema.md: Added authentication_set column but no explanation of its purpose (though self-evident from context)
   • docs/01-project/MDL_QUICK_REFERENCE.md: Minor wording improvements possible in OData authentication section

Recommendation

Approve. The changes are:

• Correctly implement the requested features per issue descriptions
• Follow the full-stack validation pattern (though these are validation/lint only, no new MDL syntax)
• Include comprehensive test coverage
• Maintain backward compatibility (especially for OData auth property refactor)
• Follow project architecture and code style guidelines
• Have appropriate documentation updates

The minor issues noted are trivial and do not affect correctness. No changes are requested.

---

Note: As this is a sync PR, the changes were previously reviewed in the fork repository. This review focuses on verifying the changes meet mxcli's contribution standards as outlined in CLAUDE.md. All validation and linting changes are properly scoped and tested.

---

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