Skip to content

Commit c0c8685

Browse files
akoclaude
andcommitted
Fix mx check tests and reconcile security on disconnect
Three issues fixed: 1. Test MDL syntax errors: missing colons in ADD ATTRIBUTE (5x), wrong CREATE ASSOCIATION syntax using (A -> B) instead of FROM A TO B (2x). 2. Tests used Execute() per statement instead of ExecuteProgram(), so ReconcileMemberAccesses never ran. Now combines steps into a single program. 3. REPL and disconnect didn't run finalization. The REPL now uses ExecuteProgram(), and execDisconnect() calls finalizeProgramExecution() before closing — ensuring security reconciliation happens regardless of execution mode. Also: findMxBinary() searches repo-local, ~/.mxcli/mxbuild/*, and PATH instead of hardcoding a single path. createTestProject() falls back to mx create-project when test source project is missing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b8cc6a1 commit c0c8685

4 files changed

Lines changed: 92 additions & 30 deletions

File tree

mdl/executor/executor.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,11 @@ func (e *Executor) execDisconnect() error {
551551
return nil
552552
}
553553

554+
// Reconcile any pending security changes before closing
555+
if err := e.finalizeProgramExecution(); err != nil {
556+
fmt.Fprintf(e.output, "Warning: finalization error: %v\n", err)
557+
}
558+
554559
e.writer.Close()
555560
fmt.Fprintf(e.output, "Disconnected from: %s\n", e.mprPath)
556561
e.writer = nil

mdl/executor/roundtrip_helpers_test.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"bytes"
1414
"io"
1515
"os"
16+
"os/exec"
1617
"path/filepath"
1718
"strings"
1819
"testing"
@@ -40,6 +41,8 @@ type testEnv struct {
4041
}
4142

4243
// copyTestProject copies the source project to a temp directory and returns the MPR path.
44+
// If the source project doesn't exist, it falls back to creating a fresh project
45+
// using `mx create-project` (requires the mx binary to be available).
4346
// The temp directory is automatically cleaned up when the test finishes.
4447
func copyTestProject(t *testing.T) string {
4548
t.Helper()
@@ -48,8 +51,10 @@ func copyTestProject(t *testing.T) string {
4851
if err != nil {
4952
t.Fatalf("Failed to resolve source project path: %v", err)
5053
}
54+
5155
if _, err := os.Stat(srcDir); os.IsNotExist(err) {
52-
t.Skipf("Source project not found: %s", srcDir)
56+
// Source project not found — try mx create-project as fallback
57+
return createTestProject(t)
5358
}
5459

5560
// Create temp directory (auto-cleaned by t.TempDir())
@@ -75,6 +80,33 @@ func copyTestProject(t *testing.T) string {
7580
return destMPR
7681
}
7782

83+
// createTestProject creates a fresh Mendix project using `mx create-project`.
84+
// Returns the path to the App.mpr file in a temp directory.
85+
func createTestProject(t *testing.T) string {
86+
t.Helper()
87+
88+
mxPath := findMxBinary()
89+
if mxPath == "" {
90+
t.Skip("mx binary not available and source project not found")
91+
}
92+
93+
destDir := t.TempDir()
94+
95+
cmd := exec.Command(mxPath, "create-project")
96+
cmd.Dir = destDir
97+
output, err := cmd.CombinedOutput()
98+
if err != nil {
99+
t.Skipf("mx create-project failed: %v\n%s", err, output)
100+
}
101+
102+
mprPath := filepath.Join(destDir, "App.mpr")
103+
if _, err := os.Stat(mprPath); os.IsNotExist(err) {
104+
t.Skipf("mx create-project did not produce App.mpr in %s", destDir)
105+
}
106+
107+
return mprPath
108+
}
109+
78110
// copyFile copies a single file from src to dst.
79111
func copyFile(src, dst string) error {
80112
in, err := os.Open(src)

mdl/executor/roundtrip_mxcheck_test.go

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,38 +3,60 @@
33
package executor
44

55
import (
6+
"fmt"
67
"os"
78
"os/exec"
89
"path/filepath"
910
"strings"
1011
"testing"
1112

1213
"github.com/mendixlabs/mxcli/mdl/ast"
14+
"github.com/mendixlabs/mxcli/mdl/visitor"
1315
)
1416

1517
// --- MX Check Integration Tests ---
1618
// These tests verify that created documents pass Mendix's validation.
1719

18-
// mxCheckPath is the path to the mx check command.
19-
const mxCheckPath = "../../reference/mxbuild/modeler/mx"
20+
// findMxBinary searches for the mx command in known locations.
21+
// Search order: reference/mxbuild/modeler/mx (repo-local), ~/.mxcli/mxbuild/*/modeler/mx
22+
// (cached downloads), PATH lookup.
23+
func findMxBinary() string {
24+
// 1. Repo-local reference path
25+
repoPath, err := filepath.Abs("../../reference/mxbuild/modeler/mx")
26+
if err == nil {
27+
if _, err := os.Stat(repoPath); err == nil {
28+
return repoPath
29+
}
30+
}
31+
32+
// 2. Cached downloads (~/.mxcli/mxbuild/*/modeler/mx)
33+
if home, err := os.UserHomeDir(); err == nil {
34+
pattern := filepath.Join(home, ".mxcli", "mxbuild", "*", "modeler", "mx")
35+
if matches, _ := filepath.Glob(pattern); len(matches) > 0 {
36+
return matches[len(matches)-1]
37+
}
38+
}
39+
40+
// 3. PATH lookup
41+
if p, err := exec.LookPath("mx"); err == nil {
42+
return p
43+
}
44+
45+
return ""
46+
}
2047

2148
// mxCheckAvailable checks if the mx command is available.
2249
func mxCheckAvailable() bool {
23-
absPath, err := filepath.Abs(mxCheckPath)
24-
if err != nil {
25-
return false
26-
}
27-
_, err = os.Stat(absPath)
28-
return err == nil
50+
return findMxBinary() != ""
2951
}
3052

3153
// runMxCheck runs mx check on the given project and returns any errors.
3254
func runMxCheck(t *testing.T, projectPath string) (string, error) {
3355
t.Helper()
3456

35-
mxPath, err := filepath.Abs(mxCheckPath)
36-
if err != nil {
37-
return "", err
57+
mxPath := findMxBinary()
58+
if mxPath == "" {
59+
return "", fmt.Errorf("mx binary not found")
3860
}
3961

4062
cmd := exec.Command(mxPath, "check", projectPath)
@@ -340,8 +362,8 @@ func TestMxCheck_CE0066_Scenarios(t *testing.T) {
340362
`CREATE MODULE ROLE ` + mod + `.S2Admin;`,
341363
`CREATE OR MODIFY PERSISTENT ENTITY ` + mod + `.S2Entity (Name: String(100));`,
342364
`GRANT ` + mod + `.S2Admin ON ` + mod + `.S2Entity (CREATE, DELETE, READ *, WRITE *);`,
343-
`ALTER ENTITY ` + mod + `.S2Entity ADD ATTRIBUTE Email String(200);`,
344-
`ALTER ENTITY ` + mod + `.S2Entity ADD ATTRIBUTE Active Boolean DEFAULT false;`,
365+
`ALTER ENTITY ` + mod + `.S2Entity ADD ATTRIBUTE Email: String(200);`,
366+
`ALTER ENTITY ` + mod + `.S2Entity ADD ATTRIBUTE Active: Boolean DEFAULT false;`,
345367
},
346368
},
347369
{
@@ -351,7 +373,7 @@ func TestMxCheck_CE0066_Scenarios(t *testing.T) {
351373
`CREATE OR MODIFY PERSISTENT ENTITY ` + mod + `.S3Parent (Name: String(100));`,
352374
`CREATE OR MODIFY PERSISTENT ENTITY ` + mod + `.S3Child (Label: String(100));`,
353375
`GRANT ` + mod + `.S3Admin ON ` + mod + `.S3Parent (CREATE, DELETE, READ *, WRITE *);`,
354-
`CREATE ASSOCIATION ` + mod + `.S3Child_S3Parent (` + mod + `.S3Child -> ` + mod + `.S3Parent);`,
376+
`CREATE ASSOCIATION ` + mod + `.S3Child_S3Parent FROM ` + mod + `.S3Child TO ` + mod + `.S3Parent;`,
355377
},
356378
},
357379
{
@@ -368,7 +390,7 @@ func TestMxCheck_CE0066_Scenarios(t *testing.T) {
368390
steps: []string{
369391
`CREATE MODULE ROLE ` + mod + `.S5Admin;`,
370392
`CREATE OR MODIFY PERSISTENT ENTITY ` + mod + `.S5Entity (Name: String(100));`,
371-
`ALTER ENTITY ` + mod + `.S5Entity ADD ATTRIBUTE Email String(200);`,
393+
`ALTER ENTITY ` + mod + `.S5Entity ADD ATTRIBUTE Email: String(200);`,
372394
`GRANT ` + mod + `.S5Admin ON ` + mod + `.S5Entity (CREATE, DELETE, READ *, WRITE *);`,
373395
},
374396
},
@@ -378,7 +400,7 @@ func TestMxCheck_CE0066_Scenarios(t *testing.T) {
378400
`CREATE MODULE ROLE ` + mod + `.S6Admin;`,
379401
`CREATE OR MODIFY PERSISTENT ENTITY ` + mod + `.S6Entity (Name: String(100));`,
380402
`GRANT ` + mod + `.S6Admin ON ` + mod + `.S6Entity (READ *);`,
381-
`ALTER ENTITY ` + mod + `.S6Entity ADD ATTRIBUTE Code String(50);`,
403+
`ALTER ENTITY ` + mod + `.S6Entity ADD ATTRIBUTE Code: String(50);`,
382404
`GRANT ` + mod + `.S6Admin ON ` + mod + `.S6Entity (CREATE, DELETE, READ *, WRITE *);`,
383405
},
384406
},
@@ -402,7 +424,7 @@ func TestMxCheck_CE0066_Scenarios(t *testing.T) {
402424
`CREATE OR MODIFY PERSISTENT ENTITY ` + mod + `.S8Entity (Name: String(100));`,
403425
`GRANT ` + mod + `.S8Admin ON ` + mod + `.S8Entity (CREATE, DELETE, READ *, WRITE *);`,
404426
`GRANT ` + mod + `.S8Viewer ON ` + mod + `.S8Entity (READ *);`,
405-
`ALTER ENTITY ` + mod + `.S8Entity ADD ATTRIBUTE Status String(50);`,
427+
`ALTER ENTITY ` + mod + `.S8Entity ADD ATTRIBUTE Status: String(50);`,
406428
},
407429
},
408430
{
@@ -412,8 +434,8 @@ func TestMxCheck_CE0066_Scenarios(t *testing.T) {
412434
`CREATE OR MODIFY PERSISTENT ENTITY ` + mod + `.S9Main (Name: String(100));`,
413435
`CREATE OR MODIFY PERSISTENT ENTITY ` + mod + `.S9Related (Value: Integer);`,
414436
`GRANT ` + mod + `.S9Admin ON ` + mod + `.S9Main (CREATE, DELETE, READ *, WRITE *);`,
415-
`ALTER ENTITY ` + mod + `.S9Main ADD ATTRIBUTE Extra String(200);`,
416-
`CREATE ASSOCIATION ` + mod + `.S9Related_S9Main (` + mod + `.S9Related -> ` + mod + `.S9Main);`,
437+
`ALTER ENTITY ` + mod + `.S9Main ADD ATTRIBUTE Extra: String(200);`,
438+
`CREATE ASSOCIATION ` + mod + `.S9Related_S9Main FROM ` + mod + `.S9Related TO ` + mod + `.S9Main;`,
417439
},
418440
},
419441
}
@@ -423,11 +445,16 @@ func TestMxCheck_CE0066_Scenarios(t *testing.T) {
423445
env := setupTestEnv(t)
424446
defer env.teardown()
425447

426-
for i, step := range sc.steps {
427-
if err := env.executeMDL(step); err != nil {
428-
if !strings.Contains(err.Error(), "already exists") {
429-
t.Fatalf("Step %d failed: %v\nMDL: %s", i+1, err, step)
430-
}
448+
// Combine all steps into a single script so ExecuteProgram
449+
// runs finalizeProgramExecution (ReconcileMemberAccesses).
450+
allMDL := strings.Join(sc.steps, "\n")
451+
prog, errs := visitor.Build(allMDL)
452+
if len(errs) > 0 {
453+
t.Fatalf("Parse failed: %v\nMDL:\n%s", errs[0], allMDL)
454+
}
455+
if err := env.executor.ExecuteProgram(prog); err != nil {
456+
if !strings.Contains(err.Error(), "already exists") {
457+
t.Fatalf("ExecuteProgram failed: %v\nMDL:\n%s", err, allMDL)
431458
}
432459
}
433460

mdl/repl/repl.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,9 @@ func (r *REPL) execute(input string) error {
220220
return nil // Don't return error, just print and continue
221221
}
222222

223-
// Execute all statements
224-
for _, stmt := range prog.Statements {
225-
if err := r.executor.Execute(stmt); err != nil {
226-
return err
227-
}
223+
// Execute all statements and run finalization (e.g. ReconcileMemberAccesses)
224+
if err := r.executor.ExecuteProgram(prog); err != nil {
225+
return err
228226
}
229227

230228
return nil

0 commit comments

Comments
 (0)