feat: ALTER WORKFLOW command with full activity manipulation

[engalar]

Summary

• Add ALTER WORKFLOW MDL command for modifying existing Mendix workflows via readPatchWrite BSON manipulation
• Support all activity types: user task, multi-user task, decision, parallel split, call microflow/workflow, jump, timers, notifications, annotations, boundary events
• Recursive activity search with @N positional disambiguation for nested activities
• 22 executor unit tests + 17 visitor tests + comprehensive MDL example (324 lines)
• BSON compatibility fixes: ServerConfiguration fields, IsReusableComponent removal, mx diff alignment

Changes

• Grammar: alterWorkflowStmt rules in MDLParser.g4/MDLLexer.g4 with OUTCOME, ACTIVITY, CONDITION tokens
• AST: AlterWorkflowStmt and operation types in mdl/ast/ast_workflow.go
• Visitor: visitor_workflow.go + refactored visitor_alter.go for ALTER dispatch
• Executor: 850-line cmd_alter_workflow.go with version gating, nil sub-doc auto-creation, boundary event warnings
• Tests: cmd_alter_workflow_test.go (22 tests), visitor_workflow_test.go (17 tests)
• Docs: MDL Quick Reference updated, CLAUDE.md implementation status updated
• BSON fixes: ServerConfiguration CustomSettings/Tracing, orphaned mxunit cleanup, mx diff compatibility

Test plan

• go build ./... passes
• go test ./mdl/executor/ ./mdl/visitor/ — 39 new tests pass
• mxcli exec alter-workflow.mdl -p app.mpr runs clean on Mendix 11.6.4 project
• User confirmed mx check passes on modified project

🤖 Generated with Claude Code

[github-actions]

AI Code Review

What Looks Good

• Full-stack implementation: The PR properly wires ALTER WORKFLOW through the complete MDL pipeline (grammar → AST → visitor → executor) with all required components present
• Comprehensive test coverage: 22 executor unit tests + 17 visitor unit tests + a 324-line MDL doctest example covering all activity types and operations
• Correct MDL syntax design: Follows ALTER pattern with qualified names, proper property format (Key: value), colons for property definitions, AS correctly used only for mappings (not seen misused in property contexts)
• Version safety: Includes proper Mendix 9.12+ version gating with actionable error messages
• BSON compatibility: Includes fixes for ServerConfiguration fields, IsReusableComponent removal, and mx diff alignment
• Documentation: Updates MDL Quick Reference, CLAUDE.md implementation status, and includes comprehensive examples
• Robust error handling: Proper handling of not-found workflows/activities, ambiguous references, and edge cases
• Validation: Test plan confirms go build, go test, mxcli exec runs clean, and mx check passes on modified projects

Recommendation

Approve - The PR is ready for merge. The ALTER WORKFLOW feature is fully implemented following all project guidelines and requirements. No blocking issues were identified. The LSP wiring requirement is not applicable as the feature doesn't add new formatting, diagnostics, or navigation targets that would require LSP changes (it modifies existing workflow entities which are already handled by existing LSP support).

---

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

[ako]

Review

What's good

• Full-stack implementation: Grammar, AST, visitor, executor, tests, docs, examples — complete pipeline per the checklist
• readPatchWrite pattern: Correctly reuses the proven ALTER PAGE approach for BSON manipulation
• @N positional disambiguation: Addresses the activity name ambiguity concern from the PR #88 design review — activities can be referenced by name with @N suffix when duplicates exist
• BSON fixes are well-motivated: CustomSettings/Tracing missing from ServerConfiguration and IsReusableComponent removal fix real mx diff crashes
• Good test coverage: 22 executor tests + 17 visitor tests
• The writer_units.go fix is important: Cleaning up orphan mxunit files on insert failure prevents corrupted projects

Concerns

Scope — system_module.go (454 lines) is unrelated. This adds a virtual System module builder with all System entities/associations defined in code. It's a substantial feature on its own. Per the review checklist: "Each PR is scoped to a single feature or concern." This should be a separate PR unless ALTER WORKFLOW specifically needs it.

Scope — BSON compatibility fixes are bundled. The writer_settings.go, writer_modules.go, and writer_units.go changes fix real bugs but are independent of ALTER WORKFLOW. These should ideally be their own commit(s) or PR for clean bisectability.

SerializeWorkflowActivity is exported for cross-package use. The writer_workflow.go change exports a previously-internal function. This adds to the public API surface — make sure it's intentional and not just a shortcut to avoid a proper internal interface.

Generated parser files include a visitor mode switch. The diff shows new mdlparser_visitor.go and mdlparser_base_visitor.go files (1185 + 1576 lines). Was the ANTLR generation changed to emit visitors in addition to listeners? This is a significant change to the parser generation and should be called out explicitly.

The plan document (1,092 lines) is included. docs/plans/2026-04-06-alter-workflow.md is the implementation plan used to build this. Should it be committed as-is, or trimmed now that the implementation is done?

Recommendation

The ALTER WORKFLOW implementation itself looks solid. I'd suggest:

1. Split out system_module.go into its own PR
2. Split out the BSON fixes (writer_settings.go, writer_modules.go, writer_units.go) into a separate commit or PR
3. Clarify the visitor-mode parser generation change

The core ALTER WORKFLOW code (grammar, AST, visitor, executor, tests, docs) is ready once the scope is cleaned up.

[github-actions]

AI Code Review

Critical Issues

• LSP/VS Code integration missing: The PR adds a new MDL statement (ALTER WORKFLOW) but shows no changes to LSP wiring (cmd/mxcli/lsp.go) or VS Code extension (vscode-mdl/package.json). While syntax highlighting will likely work from grammar changes, features like completion suggestions for activity names/properties, diagnostics for invalid workflow references, and formatting support may not function properly without explicit LSP updates.

Moderate Issues

• Expression quoting consistency in boundary events: The syntax uses quoted expressions like INTERRUPTING TIMER 'addHours([%CurrentDateTime%], 2)'. While this may be consistent with how MDL handles expressions elsewhere (as string literals to be interpreted later), it creates inconsistency with other MDL contexts where expressions appear without outer quotes (e.g., in microflow definitions). This could confuse users expecting uniform expression handling.

Minor Issues

• Manual token maintenance risk: The lexer adds three new tokens (OUTCOME, ACTIVITY, CONDITION) that must be kept in sync with the parser. While not immediately problematic, this creates a maintenance risk if future changes forget to update both files.

What Looks Good

• Full-stack implementation: All required layers are present (grammar → AST → visitor → executor) with proper version gating (Mendix 9.12+).
• Comprehensive testing: 22 executor unit tests + 17 visitor tests + a 324-line MDL example covering all activity types and edge cases.
• Thoughtful activity reference handling: Implements recursive search through nested workflow structures with @N positional disambiguation for ambiguous names.
• Robust BSON manipulation: Properly handles auto-creation of sub-documents, UUID generation, array markers, and error conditions.
• Clear documentation: MDL Quick Reference updated with syntax table and examples; CLAUDE.md implementation status updated.
• Real-world validation: User tested on Mendix 11.6.4 project and confirmed mx check passes on modified output.

Recommendation

Approve with minor follow-up items
The PR implements a substantial feature correctly and thoroughly. The LSP integration gap should be addressed to ensure full IDE support, and the expression quoting pattern warrants verification against existing MDL conventions. However, the core functionality is sound, well-tested, and ready for merging. The LSP and expression concerns can be handled in a follow-up PR if deemed necessary post-merge.

---

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

[engalar]

Thanks for the review @ako! Addressed all concerns:

1. system_module.go split out → feat: add virtual System module for complete module listing #110 (independent PR)
2. BSON fixes split out → fix: align BSON properties with Mendix schema for mx diff compatibility #109 (writer_settings.go, writer_modules.go, writer_units.go, debug-bson.md)
3. ANTLR visitor files removed — were generated without -no-visitor flag by mistake. Regenerated cleanly with make grammar.
4. Plan document removed — docs/plans/2026-04-06-alter-workflow.md dropped from this PR.
5. SerializeWorkflowActivity export — intentional, called 3 times by cmd_alter_workflow.go (lines 495, 528, 579) to serialize activities during INSERT/REPLACE operations. Required for cross-package use.

PR is now a single clean commit with only ALTER WORKFLOW code (25 files). Rebased on latest upstream/main.

[engalar]

CI failure in build-and-test is expected — alter-workflow.mdl roundtrip test fails with failed to resolve entity System.WorkflowUserTask because the System module entity resolution depends on another PR. Will pass once that PR is merged.

[ako]

Scope concerns addressed — system_module.go, BSON fixes, visitor-mode parser files, and plan document all removed. PR is now cleanly scoped to ALTER WORKFLOW only. LGTM.