From 341c0b15ad99d620fcbb16928c53b952641b7cd8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 16:00:26 +0000 Subject: [PATCH 1/8] brain: list the committed store as data `staged --json` returned entry objects, but the promoted side had only `brief --json`, which returns each shard's rendered markdown: right for a prompt, wrong for a UI, which then has to parse a format mxcli owns. - `brain list [--slice ] [--kind decision|requirement] [--open]` lists committed entries in store order. With --json each is the object `staged --json` returns plus its shard, under "entries", with "count" and "malformed". --open=false keeps only settled entries. - Blocks whose metadata line cannot be read are reported under "malformed" for the shards the filter covers, never dropped. - `kind` is always explicit, in `list` and now in `staged --json` too: a consumer must not have to know that an empty kind means a decision (and without it, --kind decision would match nothing). - --slice with --kind decision, and an unknown kind, are refused rather than matching nothing. No model is needed; no store lists nothing. Fixes ako/mxcli#1952 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011oK82sxYNXAhaYkijzRwae --- cmd/mxcli/brain/list.go | 87 +++++++++++++++++ cmd/mxcli/brain/list_test.go | 137 +++++++++++++++++++++++++++ cmd/mxcli/cmd_brain.go | 78 ++++++++++++++- cmd/mxcli/cmd_brain_list_test.go | 102 ++++++++++++++++++++ docs-site/src/tools/project-brain.md | 36 +++++++ 5 files changed, 438 insertions(+), 2 deletions(-) create mode 100644 cmd/mxcli/brain/list.go create mode 100644 cmd/mxcli/brain/list_test.go create mode 100644 cmd/mxcli/cmd_brain_list_test.go diff --git a/cmd/mxcli/brain/list.go b/cmd/mxcli/brain/list.go new file mode 100644 index 000000000..0c5b93c4f --- /dev/null +++ b/cmd/mxcli/brain/list.go @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: Apache-2.0 + +// list.go - the committed store as data. +// +// `brief --json` returns each shard's rendered markdown, which is right for a +// prompt and wrong for a UI: rendering a tree from it means parsing a format +// mxcli owns, a second parser of the same thing that drifts the first time the +// format changes (ako/mxcli#1952). List hands out the parsed entries instead. +package brain + +import ( + "fmt" + "slices" +) + +// ListFilter narrows a listing. The zero value lists everything. +type ListFilter struct { + // Slice keeps one slice's entries: its requirements and the questions + // filed against it. + Slice string + // Kind keeps decisions or requirements. An open question is a decision + // unless it was filed against a slice. + Kind Kind + // Open, when set, keeps only open questions (true) or only settled + // entries (false). + Open *bool +} + +// ListedEntry is one committed entry with the shard it is in — the same shape +// `staged --json` gives a queued entry, so a consumer reads both sides alike. +type ListedEntry struct { + Entry + Shard string `json:"shard"` +} + +// ListResult is a listing plus the blocks that could not be read. Those are +// reported rather than dropped: a UI that silently shows fewer entries than +// the file holds is the failure a structured read exists to prevent. +type ListResult struct { + Entries []ListedEntry `json:"entries"` + Malformed []string `json:"malformed"` +} + +// List returns the committed entries matching f, in store order: project.md, +// then modules, then slices, and file order within each. +func (s *Store) List(f ListFilter) (ListResult, error) { + out := ListResult{Entries: []ListedEntry{}, Malformed: []string{}} + switch f.Kind { + case "", KindDecision, KindRequirement: + default: + return out, fmt.Errorf("kind %q: use %q or %q", f.Kind, KindDecision, KindRequirement) + } + if f.Slice != "" && f.Kind == KindDecision { + return out, fmt.Errorf("a decision has no slice, so --slice with --kind %s matches nothing; drop one of them", KindDecision) + } + + shards, err := s.ListShards() + if err != nil { + return out, err + } + if f.Slice != "" { + want := PlanShard(f.Slice) + shards = slices.DeleteFunc(shards, func(sh string) bool { return sh != want }) + } + for _, sh := range shards { + entries, malformed, err := s.LoadShard(sh) + if err != nil { + return out, err + } + for _, m := range malformed { + out.Malformed = append(out.Malformed, sh+": "+m) + } + for _, e := range entries { + // Explicit, so a consumer never has to know that an empty + // kind means a decision. + e.Kind = e.EntryKind() + if f.Kind != "" && e.Kind != f.Kind { + continue + } + if f.Open != nil && e.Open != *f.Open { + continue + } + out.Entries = append(out.Entries, ListedEntry{Entry: e, Shard: sh}) + } + } + return out, nil +} diff --git a/cmd/mxcli/brain/list_test.go b/cmd/mxcli/brain/list_test.go new file mode 100644 index 000000000..634e840ab --- /dev/null +++ b/cmd/mxcli/brain/list_test.go @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: Apache-2.0 + +package brain + +import ( + "os" + "slices" + "strings" + "testing" +) + +// ako/mxcli#1952: a structured read of the committed store, so a UI renders +// entries instead of parsing markdown whose format mxcli owns. + +func listFixture(t *testing.T) (s *Store, dec, question, reqA, reqB Entry) { + t.Helper() + s = NewStore(t.TempDir()) + if _, err := s.Init(); err != nil { + t.Fatal(err) + } + dec = mustEntry(t, "Orders are committed by Finance", "@Sales.Order") + q, err := NewQuestion("Do approvers see rejected orders?", nil, "", day) + if err != nil { + t.Fatal(err) + } + question = q + reqA = mustRequirement(t, "A planner can create a visit", "01-scheduling", "@Scheduling.Visit") + reqB = mustRequirement(t, "A dispatcher can assign a visit", "02-dispatch", "@Dispatch.ACT_Assign") + if err := s.Promote(dec, "Sales"); err != nil { + t.Fatal(err) + } + if err := s.Promote(question, ProjectShard); err != nil { + t.Fatal(err) + } + promoteAll(t, s, reqA, reqB) + return +} + +func ids(r ListResult) []string { + var out []string + for _, e := range r.Entries { + out = append(out, e.ID) + } + return out +} + +func TestListFiltersTheCommittedStore(t *testing.T) { + s, dec, question, reqA, reqB := listFixture(t) + yes, no := true, false + + for _, tc := range []struct { + name string + f ListFilter + want []string + }{ + // Store order: project, modules, slices; file order within each. + {"everything", ListFilter{}, []string{question.ID, dec.ID, reqA.ID, reqB.ID}}, + {"one slice", ListFilter{Slice: "02-dispatch"}, []string{reqB.ID}}, + {"requirements", ListFilter{Kind: KindRequirement}, []string{reqA.ID, reqB.ID}}, + {"decisions", ListFilter{Kind: KindDecision}, []string{question.ID, dec.ID}}, + {"open only", ListFilter{Open: &yes}, []string{question.ID}}, + {"settled only", ListFilter{Open: &no}, []string{dec.ID, reqA.ID, reqB.ID}}, + {"no such slice", ListFilter{Slice: "99-none"}, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := s.List(tc.f) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(ids(got), tc.want) { + t.Errorf("ids = %v, want %v", ids(got), tc.want) + } + }) + } + + // Each entry carries its shard and an explicit kind — a consumer must not + // have to know that an empty kind means "decision". + got, err := s.List(ListFilter{}) + if err != nil { + t.Fatal(err) + } + for _, e := range got.Entries { + if e.Kind == "" || e.Shard == "" { + t.Errorf("%s: kind %q shard %q", e.ID, e.Kind, e.Shard) + } + } + if got.Entries[1].Shard != "Sales" || got.Entries[2].Shard != PlanShard("01-scheduling") { + t.Errorf("shards: %+v", got.Entries) + } +} + +// A block whose metadata line cannot be read is reported, not dropped — or a +// UI silently shows fewer entries than the file holds. +func TestListReportsMalformedBlocks(t *testing.T) { + s, _, _, reqA, _ := listFixture(t) + path := s.ShardPath(reqA.Shard()) + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, append(b, []byte("\n## Hand-written, no metadata\n\nSome prose.\n")...), 0o644); err != nil { + t.Fatal(err) + } + got, err := s.List(ListFilter{}) + if err != nil { + t.Fatal(err) + } + if len(got.Malformed) != 1 || !strings.Contains(got.Malformed[0], "Hand-written") || !strings.HasPrefix(got.Malformed[0], reqA.Shard()) { + t.Errorf("malformed = %v", got.Malformed) + } + if len(got.Entries) != 4 { + t.Errorf("the well-formed entries must still be listed: %v", ids(got)) + } + + // Malformed blocks are reported only for the shards the filter covers. + got, err = s.List(ListFilter{Slice: "02-dispatch"}) + if err != nil { + t.Fatal(err) + } + if len(got.Malformed) != 0 { + t.Errorf("a malformed block outside the filtered slice was reported: %v", got.Malformed) + } +} + +func TestListRefusesAFilterThatCannotMatch(t *testing.T) { + s := NewStore(t.TempDir()) + if _, err := s.List(ListFilter{Slice: "01-scheduling", Kind: KindDecision}); err == nil { + t.Error("--slice with --kind decision must be refused: a decision has no slice") + } + if _, err := s.List(ListFilter{Kind: "story"}); err == nil { + t.Error("an unknown kind must be refused, not match nothing") + } + // An uninitialised store lists nothing; it is not an error. + if got, err := s.List(ListFilter{}); err != nil || len(got.Entries) != 0 { + t.Errorf("empty store: %+v, %v", got, err) + } +} diff --git a/cmd/mxcli/cmd_brain.go b/cmd/mxcli/cmd_brain.go index 31f2afb5c..543d3aa1f 100644 --- a/cmd/mxcli/cmd_brain.go +++ b/cmd/mxcli/cmd_brain.go @@ -66,6 +66,7 @@ on 'plan' and 'check' with the same -C count against the model.`, mxcli brain brief --slice 02-approvals -p app.mpr mxcli brain plan -p app.mpr mxcli brain check -p app.mpr + mxcli brain list -p app.mpr --json mxcli brain show -p app.mpr`, } @@ -407,6 +408,71 @@ func runBrainChange(cmd *cobra.Command, id, verb string, change func(brain.Entry fmt.Printf("%s %s in %s\n", verb, id, shardLabel(moved.To)) } +var brainListCmd = &cobra.Command{ + Use: "list", + Short: "The committed entries as data, filterable (the structured read of docs/brain/)", + Long: `List the committed entries — the promoted side of what 'staged' shows. + +With --json each entry is the object 'staged --json' returns (id, kind, slice, +open, title, body, anchors, source, date) plus the shard it is in, so a tool can +render the store without parsing its markdown. 'brief --json' is for a prompt; +this is for a program. + + --slice one slice: its requirements and the questions filed on it + --kind decision or requirement + --open only open questions; --open=false only settled entries + +Blocks whose metadata line cannot be read are listed under "malformed" rather +than dropped, so a listing never silently shows fewer entries than the files +hold. No model is needed.`, + Example: ` mxcli brain list -C ./App --json + mxcli brain list --slice 01-scheduling -p app.mpr --json + mxcli brain list --open -p app.mpr`, + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + store := brain.NewStore(brainProjectDir(cmd)) + var f brain.ListFilter + f.Slice, _ = cmd.Flags().GetString("slice") + kind, _ := cmd.Flags().GetString("kind") + f.Kind = brain.Kind(kind) + if cmd.Flags().Changed("open") { + open, _ := cmd.Flags().GetBool("open") + f.Open = &open + } + res, err := store.List(f) + if err != nil { + brainFatal(err) + } + if globalJSONFlag { + brainJSON(map[string]any{ + "entries": res.Entries, + "count": len(res.Entries), + "malformed": res.Malformed, + }) + return + } + if !store.Exists() { + fmt.Println("No store yet. Create one with 'mxcli brain init'.") + return + } + for _, m := range res.Malformed { + fmt.Printf("MALFORMED %s\n", m) + } + if len(res.Entries) == 0 { + fmt.Println("No committed entries match.") + return + } + for _, e := range res.Entries { + open := "" + if e.Open { + open = " OPEN" + } + fmt.Printf("%s %-18s %s%s\n", e.ID, shardLabel(e.Shard), e.Title, open) + } + fmt.Printf("\n%d committed.\n", len(res.Entries)) + }, +} + var brainShowCmd = &cobra.Command{ Use: "show [shard]", Short: "Size and headroom per shard (computed, never written down)", @@ -1057,6 +1123,7 @@ func brainLocatedCommands() []*cobra.Command { brainInitCmd, brainCaptureCmd, brainStagedCmd, brainPromoteCmd, brainDropCmd, brainShowCmd, brainCheckCmd, brainPlanCmd, brainResolveCmd, brainBriefCmd, brainEditCmd, brainMoveCmd, + brainListCmd, } } @@ -1078,6 +1145,9 @@ func init() { brainEditCmd.Flags().StringSliceP("anchor", "a", nil, "Replace the anchors with these (repeatable); without -a the anchors are kept") brainMoveCmd.Flags().StringP("slice", "s", "", "The slice to move the requirement to") + brainListCmd.Flags().String("slice", "", "Only this slice's entries") + brainListCmd.Flags().String("kind", "", "Only decisions or only requirements (decision|requirement)") + brainListCmd.Flags().Bool("open", false, "Only open questions; --open=false for only settled entries") brainPromoteCmd.Flags().String("to", "", "Override the derived shard (use 'project' for a cross-cutting fact)") brainStagedCmd.Flags().String("since", "", @@ -1097,7 +1167,7 @@ func init() { "Modules being worked, for a session with no slice; repeatable") brainCmd.AddCommand(brainInitCmd, brainCaptureCmd, brainStagedCmd, brainPromoteCmd, brainDropCmd, brainShowCmd, brainCheckCmd, brainPlanCmd, - brainResolveCmd, brainBriefCmd, brainEditCmd, brainMoveCmd) + brainResolveCmd, brainBriefCmd, brainEditCmd, brainMoveCmd, brainListCmd) rootCmd.AddCommand(brainCmd) } @@ -1132,7 +1202,11 @@ type stagedEntry struct { func stagedReport(all, shown []brain.Entry, filter brain.StagedFilter) map[string]any { out := make([]stagedEntry, 0, len(shown)) for _, e := range shown { - out = append(out, stagedEntry{Entry: e, Shard: e.Shard()}) + shard := e.Shard() + // Explicit, as 'list' gives it: a consumer must not have to know + // that an empty kind means a decision. + e.Kind = e.EntryKind() + out = append(out, stagedEntry{Entry: e, Shard: shard}) } rep := map[string]any{"staged": out, "count": len(out)} diff --git a/cmd/mxcli/cmd_brain_list_test.go b/cmd/mxcli/cmd_brain_list_test.go new file mode 100644 index 000000000..b44a52aff --- /dev/null +++ b/cmd/mxcli/cmd_brain_list_test.go @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/cmd/mxcli/brain" +) + +type listedJSON struct { + Count int `json:"count"` + Entries []struct { + ID string `json:"id"` + Kind string `json:"kind"` + Slice string `json:"slice"` + Title string `json:"title"` + Anchors []string `json:"anchors"` + Shard string `json:"shard"` + } `json:"entries"` + Malformed []string `json:"malformed"` +} + +// ako/mxcli#1952 through the CLI: the promoted side as entry objects, the same +// shape as `staged --json`, filterable, with no model needed. +func TestBrainListGivesTheCommittedStoreAsData(t *testing.T) { + dir := t.TempDir() + + // Before init: still JSON, with an empty list rather than prose. + var got listedJSON + out := runBrainForTest(t, []string{"brain", "list", "-C", dir, "--json"}) + if err := json.Unmarshal([]byte(out), &got); err != nil || got.Count != 0 || got.Entries == nil || got.Malformed == nil { + t.Fatalf("list --json on no store: %v\n%s", err, out) + } + + runBrainForTest(t, []string{"brain", "init", "-C", dir}) + store, queue := brain.NewStore(dir), brain.NewQueue(dir) + dec, err := brain.NewEntry("Orders are committed by Finance", []string{"@Sales.Order"}, brainTestDay()) + if err != nil { + t.Fatal(err) + } + req, err := brain.NewRequirement("A planner can create a visit", []string{"@Scheduling.Visit"}, "01-scheduling", brainTestDay()) + if err != nil { + t.Fatal(err) + } + for _, e := range []brain.Entry{dec, req} { + if err := store.Promote(e, e.Shard()); err != nil { + t.Fatal(err) + } + } + // Staged, not committed: must not appear in list. + pending, err := brain.NewEntry("Invoices are numbered per year", []string{"@Sales.Invoice"}, brainTestDay()) + if err != nil { + t.Fatal(err) + } + if _, err := queue.Append(pending); err != nil { + t.Fatal(err) + } + + out = runBrainForTest(t, []string{"brain", "list", "-C", dir, "--json"}) + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("list --json: %v\n%s", err, out) + } + if got.Count != 2 || len(got.Entries) != 2 { + t.Fatalf("want the two committed entries, not the staged one:\n%s", out) + } + if e := got.Entries[0]; e.ID != dec.ID || e.Kind != "decision" || e.Shard != "Sales" || len(e.Anchors) != 1 { + t.Errorf("decision: %+v", e) + } + if e := got.Entries[1]; e.ID != req.ID || e.Kind != "requirement" || e.Slice != "01-scheduling" || e.Shard != brain.PlanShard("01-scheduling") { + t.Errorf("requirement: %+v", e) + } + + out = runBrainForTest(t, []string{"brain", "list", "-C", dir, "--slice", "01-scheduling", "--json"}) + if err := json.Unmarshal([]byte(out), &got); err != nil || got.Count != 1 || got.Entries[0].ID != req.ID { + t.Errorf("--slice: %v\n%s", err, out) + } + out = runBrainForTest(t, []string{"brain", "list", "-C", dir, "--kind", "decision", "--json"}) + if err := json.Unmarshal([]byte(out), &got); err != nil || got.Count != 1 || got.Entries[0].ID != dec.ID { + t.Errorf("--kind decision: %v\n%s", err, out) + } + + // staged --json now names the kind too, so both sides read alike. + var staged struct { + Staged []struct { + Kind string `json:"kind"` + } `json:"staged"` + } + out = runBrainForTest(t, []string{"brain", "staged", "-C", dir, "--json"}) + if err := json.Unmarshal([]byte(out), &staged); err != nil || len(staged.Staged) != 1 || staged.Staged[0].Kind != "decision" { + t.Errorf("staged --json kind: %v\n%s", err, out) + } + + // Control: without --json it prints for a person. + out = runBrainForTest(t, []string{"brain", "list", "-C", dir}) + var anyJSON map[string]any + if json.Unmarshal([]byte(out), &anyJSON) == nil || !strings.Contains(out, "2 committed.") { + t.Errorf("human output:\n%s", out) + } +} diff --git a/docs-site/src/tools/project-brain.md b/docs-site/src/tools/project-brain.md index 6f2b42323..d13fea2a9 100644 --- a/docs-site/src/tools/project-brain.md +++ b/docs-site/src/tools/project-brain.md @@ -316,6 +316,41 @@ rewrite the file. The queue is less forgiving: an older build that rewrites `.mxcli/brain/staged.jsonl` (on a capture, drop or promote) drops the source of the entries still queued there. +## Reading the store from a program + +```bash +mxcli brain list -C ./App --json +mxcli brain list --slice 01-scheduling --json -p app.mpr +mxcli brain list --open --json -p app.mpr # --open=false: only settled entries +``` + +`brain list --json` is the committed side of `brain staged --json`: the same +entry objects — `id`, `kind`, `slice`, `open`, `title`, `body`, `anchors`, +`source`, `date` — each with the `shard` it is in, in store order (`project.md`, +modules, slices; file order within each). `kind` is always present. + +```json +{ + "count": 1, + "entries": [ + { + "id": "72dfdd", "kind": "requirement", "slice": "01-scheduling", + "title": "A planner can create a visit", + "anchors": ["@Scheduling.Visit"], "source": "docs/sample-prd.md#L18-21", + "date": "2026-11-04", "shard": "plan/01-scheduling" + } + ], + "malformed": [] +} +``` + +Use it rather than parsing the shard files: their format is mxcli's to change. +`brief --json` is the other read, meant for a prompt — it returns each shard's +rendered markdown. A block whose metadata line cannot be read is listed under +`malformed` instead of being dropped, so a listing never quietly shows fewer +entries than the files hold. Built-ness is not in the listing: that is derived +from the model by `brain plan`, never stored on an entry. + ## Editing an entry ```bash @@ -425,6 +460,7 @@ progress from 65/65 to 63/65 and the number was the only symptom. | `brain plan [--slice ]` | Each slice's requirements counted against the model | | `brain brief --slice \| --module ` | The reading pack: exactly the shards that work needs | | `brain check [--changed]` | Anchors resolve, entries filed correctly, plus slice progress | +| `brain list [--slice ] [--kind ] [--open]` | The committed entries; with `--json`, as data | | `brain show []` | Entries, lines and headroom per shard | Dropping the last entry from a module shard deletes the file, so the directory From 11e748ce4877bd36a37fd9e5c9990b4a703c2848 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 16:14:26 +0000 Subject: [PATCH 2/8] fix(test): --attach on Mendix 11.14.0 says why it is refused, not "an entity changed" The nightly went red on the 11.14.0 leg only, in TestTestAttach_KeepsTheHostingAppsPageBundles (added by ako/mxcli#1917): `mxcli test --attach` exited 1 with "this change needs a runtime restart (an entity or association changed)" although it only added a test microflow. Measured over mxbuild's /build API with no watch loop: on 11.14.0 every later build in a serve process fails, even for an unchanged model (Cannot find module .../web/rollup.config.mjs), and the failed response has no restartRequired key at all. 11.15.0 builds them (false for an unchanged model and for a microflow, true for an entity), which is the 11.12.5 shape #1917 already recovers. So on 11.14.0 the attach falls back to a fresh serve process, whose cold build asks for a restart whatever changed, and the attach read that as a structural change. Before #1917 the same attach failed on the build itself, so it never worked on 11.14.0. serveRecovery now marks a fresh process's answer BuildResult.ColdRestart (rebuildFresh, and the build after a failed restart) and clears it where it takes the incremental build's answer instead. The attach's refusal names the cold answer for what it is and points to --local, since restarting the dev loop leaves the next attach in the same place. The integration test expects that refusal on 11.14.x (and still checks the pages answer 200), and fails when an 11.14 mxbuild builds incrementally again. Controls: the old test fails on 11.14.0 with the nightly's exact message; the new test passes on 11.15.0; stubbing the ColdRestart mark fails the unit tests that pin it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GeAhBjRBm5vN2ik3ddvVWW --- ...only-test-attach-refused-cold-restart.json | 1 + .../skills/mendix/test-microflows/SKILL.md | 20 ++++++----- CHANGELOG.md | 1 + ...cmd_test_attach_bundle_integration_test.go | 35 +++++++++++++++---- cmd/mxcli/docker/mxserve.go | 9 +++++ cmd/mxcli/docker/serve_attached_test.go | 34 ++++++++++++++++++ cmd/mxcli/docker/serve_recovery.go | 17 +++++++-- cmd/mxcli/docker/serve_recovery_test.go | 13 +++++-- cmd/mxcli/testrunner/runner_attach.go | 25 ++++++++++--- .../testrunner/runner_attach_restart_test.go | 32 +++++++++++++++++ docs-site/src/tools/running-tests.md | 7 ++++ 11 files changed, 170 insertions(+), 24 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-nightly-11-14-only-test-attach-refused-cold-restart.json create mode 100644 cmd/mxcli/testrunner/runner_attach_restart_test.go diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-nightly-11-14-only-test-attach-refused-cold-restart.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-nightly-11-14-only-test-attach-refused-cold-restart.json new file mode 100644 index 000000000..996b4723a --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-nightly-11-14-only-test-attach-refused-cold-restart.json @@ -0,0 +1 @@ +{"area":"cmd/mxcli","date":"2026-11-04","symptom":"The nightly goes red on the Mendix 11.14.0 leg only, in `TestTestAttach_KeepsTheHostingAppsPageBundles` (added the day before by ako/mxcli#1917): `mxcli test --attach` exits 1 with `this change needs a runtime restart (an entity or association changed), which --attach cannot do`, although the attach only adds a test microflow. 11.12.x, 11.13, 11.15 and 11.16 are green","cause":"On 11.14.0 every later build in an mxbuild --serve process fails, even with the model unchanged (`Cannot find module …/web/rollup.config.mjs`), and the failed /build response carries no restartRequired key at all. #1917 routes the attach's build through serveRecovery, whose legacy-hint path restarts onto a private serve and builds cold; a cold build reports restartRequired=true whatever changed, and runner_attach read that as a structural change. Before #1917 the same attach failed with the build failure itself, so attach never worked on 11.14.0; the test exposed it because its default 11.12.6 falls back to whatever mxbuild the nightly leg caches","file":"`cmd/mxcli/docker/serve_recovery.go` (rebuildFresh marks the cold answer `BuildResult.ColdRestart`; the incomplete-bundle path clears it when it takes the incremental answer), `cmd/mxcli/testrunner/runner_attach.go` (`attachRestartRefusal`), `cmd/mxcli/cmd_test_attach_bundle_integration_test.go` (11.14 branch)","insight":"A recovery that substitutes a fresh process's answer must say so: a cold build's restartRequired is \"unknown\", and only a caller that can restart may treat it as true. Settled by driving /build directly (a throwaway test with StartServe + Build, no watch loop): 11.14.0 build 2 with no change = Failure with no restartRequired; 11.15.0 = Success, false for no change and for a microflow, true for an entity, 0 pages (the 11.12.5 shape #1917 already recovers). That table is what puts the limit on 11.14.0 alone, so the test pins `>= 11.14 && < 11.15` and fails when an 11.14 mxbuild builds incrementally again. Reproduce in about 4 minutes: `mxcli setup mxbuild --version 11.14.0`, JAVA_HOME at a JDK 25, `MXCLI_RUNLIFE_VERSION=11.14.0 go test -tags integration -run TestTestAttach_KeepsTheHostingAppsPageBundles ./cmd/mxcli/`. Controls: the old test fails with the nightly's exact message; the new test passes on 11.14.0 and 11.15.0 and fails on 11.14.0 against a binary without the fix (it names the entity); stubbing the ColdRestart mark fails the two unit tests that pin it","refs":["mendixlabs/mxcli nightly run 41801646309","ako/mxcli nightly run 41801123003","ako/mxcli#1917","mendixlabs/mxcli#1723"],"test":"cmd/mxcli/cmd_test_attach_bundle_integration_test.go; cmd/mxcli/docker/serve_attached_test.go; cmd/mxcli/docker/serve_recovery_test.go; cmd/mxcli/testrunner/runner_attach_restart_test.go"} diff --git a/.claude/skills/mendix/test-microflows/SKILL.md b/.claude/skills/mendix/test-microflows/SKILL.md index aec814a13..5abcff80a 100644 --- a/.claude/skills/mendix/test-microflows/SKILL.md +++ b/.claude/skills/mendix/test-microflows/SKILL.md @@ -16,14 +16,14 @@ description: "Write and run MDL-based microflow tests with `mxcli test` — anno | 185-227 | Test File Formats | | 228-243 | string Operations | | 244-267 | Running Tests | -| 268-461 | How It Works | -| 462-500 | Check a test file before you run it | -| 501-622 | Writing Good Tests | -| 623-637 | Test File Organization | -| 638-650 | Interpreting Failures | -| 651-672 | CI Integration | -| 673-679 | Related Skills | -| 680-682 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | +| 268-465 | How It Works | +| 466-504 | Check a test file before you run it | +| 505-626 | Writing Good Tests | +| 627-641 | Test File Organization | +| 642-654 | Interpreting Failures | +| 655-676 | CI Integration | +| 677-683 | Related Skills | +| 684-686 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | # Test Microflows Skill @@ -389,6 +389,10 @@ Three things to know before reaching for it: - **A change needing a runtime restart is refused** — a new entity or association. That runtime belongs to the other process. Restart it, or drop `--attach`. +- **On Mendix 11.14.0 every attach is refused**, whatever the tests change: its + `mxbuild --serve` fails every build after its first, so the attach falls back + to a full build, and a full build asks for a runtime restart whatever changed. + Use `--local` there. (11.12.x, 11.13 and 11.15+ build incrementally.) | | Boot | Database | Owns the runtime | |---|---|---|---| diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fc07b4aa..d256adf30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`test --attach` on Mendix 11.14.0 says why it is refused instead of blaming an entity** — that version's `mxbuild --serve` fails every build after its first, so since ako/mxcli#1917 the attach builds on a fresh serve process, whose first build asks for a runtime restart whatever changed. The refusal claimed "an entity or association changed" and suggested restarting the dev loop, which does not help; it now says the build could not be done incrementally and points to `--local`. The hosting app's pages survive either way. 11.15.0 builds incrementally and attaches as before. - **`describe microflow` no longer leaves out a note linked to the start event or to another note** (ako/mxcli#1933, mendixlabs/mxcli#1731) — such a note counted as linked, so it was not printed as a free note, and the element it is linked to prints no `@annotation`, so it was not printed at all; re-executing the description deleted it. It is now described as a free note, after a `-- WARNING` line saying the link itself has no MDL form and a rewrite drops it. - **Rebuilding a described decision no longer adds a merge** (ako/mxcli#1929, mendixlabs/mxcli#1727) — a `case`, `split type` or `if … elsif` one of whose branches returns is described with its own merge as a label (`end case; join shared1; merge shared1;`). Executing that as a fresh create built the decision's merge and a second one for the label, wired merge to merge, and every further describe → drop + create added another (`join junctionN; merge junctionN;`). The label's merge is now the decision's merge when nothing else reaches it, so the round trip keeps one merge. - **`describe microflow` prints a blank `call external action` argument as `= nothing`** (ako/mxcli#1915) — Studio Pro 11.16 stores an optional OData action parameter left blank as an empty argument, and `describe` printed it as `additional = )`, which does not parse, so every `mdl 1;` `create or modify` of such a microflow was refused (MDL-V1-REBUILD). `call external action` now takes `Param = nothing`, the blank-argument spelling `call microflow` and `call nanoflow` already use; `Param = empty` (what 11.14 stored) stays a different value, and each round-trips as itself. diff --git a/cmd/mxcli/cmd_test_attach_bundle_integration_test.go b/cmd/mxcli/cmd_test_attach_bundle_integration_test.go index efaa08fe2..d9f5277db 100644 --- a/cmd/mxcli/cmd_test_attach_bundle_integration_test.go +++ b/cmd/mxcli/cmd_test_attach_bundle_integration_test.go @@ -29,6 +29,14 @@ import ( // // The control is the same page module before the attach, which the boot's // first (complete) build serves with a 200. +// +// On Mendix 11.14.0 the attach cannot apply at all: every later build in a serve +// process fails, even for an unchanged model, and the failed response carries no +// restartRequired (measured over mxbuild's /build API; 11.15.0 builds them and +// answers false for a microflow). The only answer is a cold build's, which asks +// for a restart whatever changed, so the attach is refused and must say why +// rather than claim an entity changed. Its pages must survive all the same. When +// an 11.14 mxbuild builds incrementally again, that branch fails and should go. func TestTestAttach_KeepsTheHostingAppsPageBundles(t *testing.T) { version := os.Getenv("MXCLI_RUNLIFE_VERSION") if version == "" { @@ -57,6 +65,7 @@ func TestTestAttach_KeepsTheHostingAppsPageBundles(t *testing.T) { t.Skipf("mx create-project: %v\n%s", err, out) } mpr := filepath.Join(work, "App.mpr") + coldAnswerOnly := projectAtLeast(t, mpr, 11, 14) && !projectAtLeast(t, mpr, 11, 15) const appPort, adminPort, servePort = 8487, 8497, 6943 for _, p := range []int{appPort, adminPort, servePort} { @@ -64,15 +73,20 @@ func TestTestAttach_KeepsTheHostingAppsPageBundles(t *testing.T) { t.Skipf("port %d is in use", p) } } - mxcli := func(args ...string) string { + run := func(args ...string) (string, error) { cmd := exec.Command(bin, args...) cmd.Dir = work var out bytes.Buffer cmd.Stdout, cmd.Stderr = &out, &out - if err := cmd.Run(); err != nil { - t.Fatalf("mxcli %v: %v\n%s", args, err, out.String()) + err := cmd.Run() + return out.String(), err + } + mxcli := func(args ...string) string { + out, err := run(args...) + if err != nil { + t.Fatalf("mxcli %v: %v\n%s", args, err, out) } - return out.String() + return out } t.Cleanup(func() { _ = exec.Command(bin, "run", "stop", "-p", mpr).Run() }) @@ -102,9 +116,16 @@ func TestTestAttach_KeepsTheHostingAppsPageBundles(t *testing.T) { if err := os.WriteFile(test, []byte(src), 0o644); err != nil { t.Fatal(err) } - out := mxcli("test", test, "-p", mpr, "--attach", "--skip-app-startup", "--require-assertions") - if !strings.Contains(out, "Passed: 1") { - t.Fatalf("the attach run did not pass:\n%s", out) + out, err := run("test", test, "-p", mpr, "--attach", "--skip-app-startup", "--require-assertions") + switch { + case coldAnswerOnly: + if err == nil || !strings.Contains(out, "could not build this change incrementally, and the full build") || + strings.Contains(out, "an entity or association changed") { + t.Fatalf("Mendix 11.14: want the attach refused for want of an incremental answer, not for an entity "+ + "change; if this mxbuild builds incrementally again, drop the 11.14 branch (err %v):\n%s", err, out) + } + case err != nil || !strings.Contains(out, "Passed: 1"): + t.Fatalf("the attach run did not pass (err %v):\n%s", err, out) } if got := status(); got != http.StatusOK { diff --git a/cmd/mxcli/docker/mxserve.go b/cmd/mxcli/docker/mxserve.go index 66c4ecacf..925c22586 100644 --- a/cmd/mxcli/docker/mxserve.go +++ b/cmd/mxcli/docker/mxserve.go @@ -78,6 +78,15 @@ type BuildResult struct { Message string `json:"message"` Problems BuildProblems `json:"problems"` Raw json.RawMessage `json:"-"` + + // ColdRestart marks a RestartRequired that comes from a fresh serve + // process's first build with no incremental build to correct it. A first + // build reports true even for an unchanged model, so here true means "could + // not tell", not "an entity changed". serveRecovery sets it; mxbuild never + // sends it. Measured on 11.14.0: every later build in a serve process + // fails, even for an unchanged model, and the failed response has no + // restartRequired at all, so this is the only answer a build there gets. + ColdRestart bool `json:"-"` } // BuildProblems is the serve response's problems object. The consistency errors diff --git a/cmd/mxcli/docker/serve_attached_test.go b/cmd/mxcli/docker/serve_attached_test.go index 305bbe669..d355f0bf7 100644 --- a/cmd/mxcli/docker/serve_attached_test.go +++ b/cmd/mxcli/docker/serve_attached_test.go @@ -107,6 +107,40 @@ func TestAttachedServe_CompletesTheBundleOnItsOwnServe(t *testing.T) { } } +// Mendix 11.14.0: the host's build fails outright, even for an unchanged model, +// and the failure carries no restartRequired (measured over mxbuild's /build +// API). The private serve's cold build is then the only answer, and it asks for +// a restart whatever changed: the result says it is that answer, so the attach +// can refuse without blaming an entity. +func TestAttachedServe_FailedHostBuildLeavesOnlyAColdAnswer(t *testing.T) { + deploy := prebuiltDeployment(t) + config := filepath.Join(deploy, "web", "rollup.config.mjs") + host := &scriptedServe{build: func(_, _ int) (*BuildResult, error) { + return &BuildResult{Status: "Failure", Message: "Compilation of the app bundle failed.", + Raw: []byte(`{"problems":{"errors":[{"details":"Error [ERR_MODULE_NOT_FOUND]: Cannot find module '` + + config + `' imported from rollup-runner.mjs"}]},"status":"Failure"}`)}, nil + }} + var own *privateServe + a := attachedServeFor(host, deploy, func() (ownedServe, error) { + own = newPrivateServe(t, deploy) + return own, nil + }) + res, err := a.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}) + if err != nil || !res.OK() { + t.Fatalf("Build = %+v, %v", res, err) + } + if !res.RestartRequired || !res.ColdRestart { + t.Fatalf("want the cold build's restartRequired, marked ColdRestart; got %+v", res) + } + if host.restarts != 0 || own == nil { + t.Fatalf("host restarts = %d, private serve started = %v; want only a private one", host.restarts, own != nil) + } + if pagesIn(t, deploy) == 0 { + t.Fatal("dist/pages is empty after the attach's build") + } + a.Close() +} + // Control: a host build that leaves the bundle complete is all there is — no // private serve process is started for it. func TestAttachedServe_CompleteHostBuildStartsNothing(t *testing.T) { diff --git a/cmd/mxcli/docker/serve_recovery.go b/cmd/mxcli/docker/serve_recovery.go index bef1a4720..cba890248 100644 --- a/cmd/mxcli/docker/serve_recovery.go +++ b/cmd/mxcli/docker/serve_recovery.go @@ -77,6 +77,7 @@ type serveRecovery struct { } func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { + freshProcess := r.restartPending if r.restartPending { fmt.Fprintln(r.out, " starting mxbuild --serve (the last restart failed)...") if err := r.restart(); err != nil { @@ -89,6 +90,9 @@ func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { if err != nil { return res, err } + if freshProcess && res.OK() { + res.ColdRestart = true // the first build of the process just started + } staleTheme := r.themeFailed && isThemeFailure(res) r.observe(res) @@ -122,7 +126,7 @@ func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { // succeeded does, and its model is the one the cold build rewrote // byte-for-byte; taking the cold answer would restart the runtime — and // drop every browser session — on every change. - fresh.RestartRequired = res.RestartRequired + fresh.RestartRequired, fresh.ColdRestart = res.RestartRequired, false return fresh, nil } @@ -130,13 +134,17 @@ func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { fmt.Fprintln(r.out, " mxbuild --serve cannot build this change incrementally (it wants client source its "+ "first build does not leave behind); restarting mxbuild --serve for a full build...") // No incremental build succeeded, so the cold build's restartRequired - // is the only answer there is. + // is the only answer there is, and it is marked ColdRestart: a caller + // that cannot restart the runtime must not read it as "an entity + // changed". return r.rebuildFresh(req) } return res, nil } -// rebuildFresh replaces the serve process and builds once on the new one. +// rebuildFresh replaces the serve process and builds once on the new one. Its +// RestartRequired is a first build's, so it is marked ColdRestart; a caller +// that has an incremental answer replaces both. func (r *serveRecovery) rebuildFresh(req BuildRequest) (*BuildResult, error) { if err := r.restart(); err != nil { return nil, err @@ -144,6 +152,9 @@ func (r *serveRecovery) rebuildFresh(req BuildRequest) (*BuildResult, error) { res, err := r.serve.Build(req) if err == nil { r.observe(res) + if res != nil && res.OK() { + res.ColdRestart = true + } } return res, err } diff --git a/cmd/mxcli/docker/serve_recovery_test.go b/cmd/mxcli/docker/serve_recovery_test.go index 3306e4bcc..b0d28cb81 100644 --- a/cmd/mxcli/docker/serve_recovery_test.go +++ b/cmd/mxcli/docker/serve_recovery_test.go @@ -132,8 +132,8 @@ func TestServeRecovery_IncompletePrebuiltBundleRestartsServe(t *testing.T) { // The fresh process says restartRequired for an unchanged model; the // incremental build's answer is the one that knows what changed. Taking the // cold one would restart the runtime — and drop every session — per change. - if res.RestartRequired { - t.Fatal("RestartRequired must be the incremental build's (false), not the cold build's") + if res.RestartRequired || res.ColdRestart { + t.Fatalf("RestartRequired must be the incremental build's (false), not the cold build's: %+v", res) } if got := out.String(); !strings.Contains(got, "dist/pages") || !strings.Contains(got, "mxbuild --serve") { t.Fatalf("the recovery must be reported in one line naming what was lost, got:\n%s", got) @@ -195,6 +195,12 @@ func TestServeRecovery_MissingClientSourceFailureRestartsServe(t *testing.T) { if !res.RestartRequired { t.Fatal("with no successful incremental build to say otherwise, the cold build's restartRequired stands") } + // ...but it is a first build's answer, true whatever changed, and a caller + // that cannot restart the runtime (test --attach) must be able to say so + // rather than claim an entity changed. + if !res.ColdRestart { + t.Fatal("the cold build's restartRequired must be marked ColdRestart") + } if f.restarts != 1 { t.Fatalf("restarts = %d, want 1", f.restarts) } @@ -225,6 +231,9 @@ func TestServeRecovery_FailedRestartIsRetriedOnNextBuild(t *testing.T) { if f.restarts != before+1 { t.Fatalf("the next build must start a serve process first, restarts %d -> %d", before, f.restarts) } + if !res.ColdRestart { + t.Fatal("the first build of the process just started is a cold answer and must be marked ColdRestart") + } } // themeCompileFailure is the first Sass failure in a serve process, as diff --git a/cmd/mxcli/testrunner/runner_attach.go b/cmd/mxcli/testrunner/runner_attach.go index f6e75d6eb..5c5334536 100644 --- a/cmd/mxcli/testrunner/runner_attach.go +++ b/cmd/mxcli/testrunner/runner_attach.go @@ -94,19 +94,36 @@ func (a *attachedApp) applyModelChange(projectPath string) (string, error) { return "", buildFailure(build) } // No restart callback: the runtime belongs to the other process. A structural - // change is refused rather than half-applied — see the error below. + // change is refused rather than half-applied — see attachRestartRefusal. action, err := a.ctrl.ApplyBuild(build, nil) if err != nil { return action.String(), err } if action == docker.ActionRestart { - return action.String(), fmt.Errorf("this change needs a runtime restart (an entity or association changed), " + - "which --attach cannot do — the runtime belongs to the 'mxcli run --local' process.\n" + - " Restart that process, or drop --attach to run against a scratch runtime.") + return action.String(), attachRestartRefusal(build) } return action.String(), nil } +// attachRestartRefusal is why an attach cannot apply a build that asks for a +// runtime restart. The runtime belongs to the other process, so a structural +// change is refused rather than half-applied. +func attachRestartRefusal(build *docker.BuildResult) error { + if build.ColdRestart { + // Nothing says an entity changed: the serve process could not build + // incrementally (Mendix 11.14.0 fails every later build), and a first + // build asks for a restart whatever changed. Restarting the dev loop does + // not help either — the attach's build is again a later one in its serve. + return fmt.Errorf("mxbuild --serve could not build this change incrementally, and the full build it " + + "fell back to asks for a runtime restart whatever changed (it has no earlier build to compare with), " + + "which --attach cannot do — the runtime belongs to the 'mxcli run --local' process.\n" + + " Drop --attach to run against a scratch runtime.") + } + return fmt.Errorf("this change needs a runtime restart (an entity or association changed), " + + "which --attach cannot do — the runtime belongs to the 'mxcli run --local' process.\n" + + " Restart that process, or drop --attach to run against a scratch runtime.") +} + // runAttached injects the test microflows into the already-running app, runs the // suite, and removes them again. // diff --git a/cmd/mxcli/testrunner/runner_attach_restart_test.go b/cmd/mxcli/testrunner/runner_attach_restart_test.go new file mode 100644 index 000000000..316bcff3a --- /dev/null +++ b/cmd/mxcli/testrunner/runner_attach_restart_test.go @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" +) + +// On Mendix 11.14.0 an attach's build only ever gets a cold build's answer, +// which asks for a restart whatever changed. The refusal must say that, not +// claim an entity changed, and must not offer restarting the dev loop, which +// leaves the next attach exactly where this one is. +func TestAttachRestartRefusal_ColdAnswerBlamesNoEntity(t *testing.T) { + msg := attachRestartRefusal(&docker.BuildResult{Status: "Success", RestartRequired: true, ColdRestart: true}).Error() + if strings.Contains(msg, "entity") || strings.Contains(msg, "Restart that process") { + t.Errorf("a cold build's restart answer is not evidence of an entity change:\n%s", msg) + } + if !strings.Contains(msg, "could not build this change incrementally") || !strings.Contains(msg, "Drop --attach") { + t.Errorf("want why the answer is unknown and the way out:\n%s", msg) + } +} + +// Control: an incremental build that asks for a restart did see a structural change. +func TestAttachRestartRefusal_IncrementalAnswerNamesTheChange(t *testing.T) { + msg := attachRestartRefusal(&docker.BuildResult{Status: "Success", RestartRequired: true}).Error() + if !strings.Contains(msg, "an entity or association changed") { + t.Errorf("want the structural-change refusal:\n%s", msg) + } +} diff --git a/docs-site/src/tools/running-tests.md b/docs-site/src/tools/running-tests.md index 4a903b24c..d953d66af 100644 --- a/docs-site/src/tools/running-tests.md +++ b/docs-site/src/tools/running-tests.md @@ -176,6 +176,13 @@ data behind in the app you are looking at. It needs the dev loop to have been started with `--test-endpoint`, because the endpoint's handler is registered by the after-startup microflow and cannot be added to an app that is already up. +On **Mendix 11.14.0** an attach is always refused. That version's +`mxbuild --serve` fails every build after its first, even for an unchanged model, +and says nothing about whether the runtime needs a restart. So the attach builds +on a fresh serve process instead, and a fresh process's first build asks for a +restart whatever changed — which an attach cannot do, the runtime being the other +process's. Use `--local` on 11.14.0; 11.15.0 builds incrementally again. + ## How tests execute **`--local` — the test endpoint.** One microflow is generated per test, plus a From 66e857329e781daba852f40d0de81a0e74888769 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 21:11:55 +0000 Subject: [PATCH 3/8] fix(check): resolve a call against the live flow, not its excluded twin (#1928) With a live and an excluded microflow of one name, check --references validated a call against the excluded twin's parameters and refused a correct call. The validators index stored documents by name in maps, so the twin listed last won. Each index now iterates live first and keeps the first entry: flow signatures, microflow return types, page parameter names and entities, the workflow call parameter map, and the OData microflow index. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011oK82tQYYsMeM1r2ihkNch --- ...-validator-indexes-hold-excluded-twin.json | 1 + ...-check-resolves-live-twin-not-excluded.mdl | 15 +++ mdl/executor/cmd_odata_actions.go | 7 +- mdl/executor/helpers.go | 5 +- mdl/executor/validate_datasource_args.go | 16 ++- .../validate_excluded_twin_signature_test.go | 124 ++++++++++++++++++ mdl/executor/validate_show_page_params.go | 8 +- mdl/executor/validate_workflow_refs.go | 7 +- .../validate_workflow_task_signature.go | 8 +- 9 files changed, 176 insertions(+), 15 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-11-04-validator-indexes-hold-excluded-twin.json create mode 100644 mdl-examples/bug-tests/1928-check-resolves-live-twin-not-excluded.mdl create mode 100644 mdl/executor/validate_excluded_twin_signature_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-validator-indexes-hold-excluded-twin.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-validator-indexes-hold-excluded-twin.json new file mode 100644 index 000000000..4f439af24 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-validator-indexes-hold-excluded-twin.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-04","symptom":"with a live and an excluded microflow of one name, `check --references` refused `call microflow M.Test(param1 = …)` with `has no parameter \"param1\" (declared parameters: param2)` — the excluded twin's parameters (mendixlabs/mxcli#1726)","cause":"the validators index stored documents by name in maps (`result[name] = …` over ListMicroflows/ListPages), so the twin listed LAST won: buildFlowSignatures, buildMicroflowReturnTypes, buildPageParamNames, buildPageParamEntities, the workflow call-microflow parameter map and microflowsByQualifiedName","file":"mdl/executor/validate_datasource_args.go, helpers.go, validate_show_page_params.go, validate_workflow_task_signature.go, validate_workflow_refs.go, cmd_odata_actions.go; test validate_excluded_twin_signature_test.go","insight":"The #1469 sweep fixed first-match LOOPS (`for … { if name matches { act; return } }`), where the excluded twin wins by being listed FIRST; a name-keyed MAP is the mirror image — the twin wins by being listed LAST — so #1469's excluded-first fixtures could not see it. Grep `GetQualifiedName(.*ContainerID, .*Name)\\] = ` for value maps (boolean name sets are safe), iterate liveFirst and keep the first entry. Test both list orders; the excluded-last case is the reported one. Control: without the change, the excluded-last subtests fail with the reporter's exact message.","refs":["ako/mxcli#1928","mendixlabs/mxcli#1726","mendixlabs/mxcli#1469"]} diff --git a/mdl-examples/bug-tests/1928-check-resolves-live-twin-not-excluded.mdl b/mdl-examples/bug-tests/1928-check-resolves-live-twin-not-excluded.mdl new file mode 100644 index 000000000..5c690ecdb --- /dev/null +++ b/mdl-examples/bug-tests/1928-check-resolves-live-twin-not-excluded.mdl @@ -0,0 +1,15 @@ +-- ako/mxcli#1928 (mendixlabs/mxcli#1726): `check --references` validated a call +-- against an EXCLUDED microflow when a live one has the same name, and refused +-- a correct call: "microflow BugTest1928.Test has no parameter "param1" +-- (declared parameters: param2)". +-- +-- Setup in Studio Pro (or with mxcli on a copy): BugTest1928.Test($param1) live, +-- and a second BugTest1928.Test($param2) marked "Exclude from project", listed +-- after it. Mendix accepts the pair (CE0122 counts active documents only). +-- Then `mxcli check this.mdl -p app.mpr --references` passes; naming param2 +-- instead is refused against the live flow's parameter list. +mdl 1; +create or modify microflow BugTest1928.Test2 () +begin + call microflow BugTest1928.Test(param1 = 'test'); +end; diff --git a/mdl/executor/cmd_odata_actions.go b/mdl/executor/cmd_odata_actions.go index f6447ad91..71b843611 100644 --- a/mdl/executor/cmd_odata_actions.go +++ b/mdl/executor/cmd_odata_actions.go @@ -140,8 +140,11 @@ func microflowsByQualifiedName(ctx *ExecContext) (map[string]*microflows.Microfl return nil, mdlerrors.NewBackend("list microflows", err) } out := make(map[string]*microflows.Microflow, len(mfs)) - for _, mf := range mfs { - out[strings.ToLower(h.GetQualifiedName(mf.ContainerID, mf.Name))] = mf + for _, mf := range liveFirst(mfs, microflowExcluded) { + key := strings.ToLower(h.GetQualifiedName(mf.ContainerID, mf.Name)) + if _, seen := out[key]; !seen { // live before an excluded twin (#1928) + out[key] = mf + } } return out, nil } diff --git a/mdl/executor/helpers.go b/mdl/executor/helpers.go index 9b89b1893..b38e2ec23 100644 --- a/mdl/executor/helpers.go +++ b/mdl/executor/helpers.go @@ -669,8 +669,11 @@ func buildMicroflowReturnTypes(ctx *ExecContext) map[string]string { if err != nil { return result } - for _, mf := range mfs { + for _, mf := range liveFirst(mfs, microflowExcluded) { qn := h.GetQualifiedName(mf.ContainerID, mf.Name) + if _, seen := result[qn]; seen { + continue // an excluded twin of a live flow (#1928) + } if mf.ReturnType == nil { result[qn] = "" continue diff --git a/mdl/executor/validate_datasource_args.go b/mdl/executor/validate_datasource_args.go index 38cba040b..157221579 100644 --- a/mdl/executor/validate_datasource_args.go +++ b/mdl/executor/validate_datasource_args.go @@ -631,21 +631,25 @@ func buildFlowSignatures(ctx *ExecContext) map[string]*flowSignature { return result } if mfs, err := ctx.Backend.ListMicroflows(); err == nil { - for _, mf := range mfs { + for _, mf := range liveFirst(mfs, microflowExcluded) { if mf == nil { continue } - result[strings.ToLower(h.GetQualifiedName(mf.ContainerID, mf.Name))] = - sdkFlowSignature(mf.Parameters, mf.ReturnType) + key := strings.ToLower(h.GetQualifiedName(mf.ContainerID, mf.Name)) + if _, seen := result[key]; !seen { + result[key] = sdkFlowSignature(mf.Parameters, mf.ReturnType) + } } } if nfs, err := ctx.Backend.ListNanoflows(); err == nil { - for _, nf := range nfs { + for _, nf := range liveFirst(nfs, nanoflowExcluded) { if nf == nil { continue } - result[strings.ToLower(h.GetQualifiedName(nf.ContainerID, nf.Name))] = - sdkFlowSignature(nf.Parameters, nf.ReturnType) + key := strings.ToLower(h.GetQualifiedName(nf.ContainerID, nf.Name)) + if _, seen := result[key]; !seen { + result[key] = sdkFlowSignature(nf.Parameters, nf.ReturnType) + } } } return result diff --git a/mdl/executor/validate_excluded_twin_signature_test.go b/mdl/executor/validate_excluded_twin_signature_test.go new file mode 100644 index 000000000..33fc022e9 --- /dev/null +++ b/mdl/executor/validate_excluded_twin_signature_test.go @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1928 (mendixlabs/mxcli#1726): two microflows of one qualified name, +// one excluded — which Mendix accepts (CE0122 counts active documents only). +// `check --references` validated a call against the excluded twin's parameters +// and refused a correct call. The validators index stored documents by name in +// a map, so the twin listed LAST won. Each index must hold the live one, in +// either list order. +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +type twinOrder struct { + name string + excludedFirst bool +} + +var twinOrders = []twinOrder{{"excluded twin listed last", false}, {"excluded twin listed first", true}} + +func twinPair[T any](live, excluded T, excludedFirst bool) []T { + if excludedFirst { + return []T{excluded, live} + } + return []T{live, excluded} +} + +func newExcludedTwinCtx(t *testing.T, order twinOrder) *ExecContext { + t.Helper() + mod := mkModule("Examples") + live := mkMicroflow(mod.ID, "Test") + live.Parameters = []*microflows.MicroflowParameter{{Name: "param1", Type: µflows.StringType{}}} + live.ReturnType = µflows.BooleanType{} + excl := mkMicroflow(mod.ID, "Test") + excl.Excluded = true + excl.Parameters = []*microflows.MicroflowParameter{{Name: "param2", Type: µflows.StringType{}}} + excl.ReturnType = µflows.StringType{} + + nlive := mkNanoflow(mod.ID, "NTest") + nlive.Parameters = []*microflows.MicroflowParameter{{Name: "param1", Type: µflows.StringType{}}} + nexcl := mkNanoflow(mod.ID, "NTest") + nexcl.Excluded = true + nexcl.Parameters = []*microflows.MicroflowParameter{{Name: "param2", Type: µflows.StringType{}}} + + plive := mkPage(mod.ID, "Edit") + plive.Parameters = []*pages.PageParameter{{Name: "param1", EntityName: "Examples.Live"}} + pexcl := mkPage(mod.ID, "Edit") + pexcl.Excluded = true + pexcl.Parameters = []*pages.PageParameter{{Name: "param2", EntityName: "Examples.Excluded"}} + + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return twinPair(live, excl, order.excludedFirst), nil + }, + ListNanoflowsFunc: func() ([]*microflows.Nanoflow, error) { + return twinPair(nlive, nexcl, order.excludedFirst), nil + }, + ListPagesFunc: func() ([]*pages.Page, error) { + return twinPair(plive, pexcl, order.excludedFirst), nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + return ctx +} + +func TestExcludedTwin_ValidatorIndexesHoldTheLiveDocument(t *testing.T) { + for _, order := range twinOrders { + t.Run(order.name, func(t *testing.T) { + ctx := newExcludedTwinCtx(t, order) + + sigs := buildFlowSignatures(ctx) + for _, key := range []string{"examples.test", "examples.ntest"} { + if got := sigs[key].paramNames(); len(got) != 1 || got[0] != "param1" { + t.Errorf("buildFlowSignatures[%s] params = %v, want the live [param1]", key, got) + } + } + if got := buildMicroflowReturnTypes(ctx)["Examples.Test"]; got != (µflows.BooleanType{}).GetTypeName() { + t.Errorf("buildMicroflowReturnTypes = %q, want the live flow's Boolean", got) + } + if got := buildPageParamNames(ctx)["examples.edit"]; len(got) != 1 || got[0] != "param1" { + t.Errorf("buildPageParamNames = %v, want the live [param1]", got) + } + if got := buildPageParamEntities(ctx)["examples.edit"]; len(got) != 1 || got[0] != "Examples.Live" { + t.Errorf("buildPageParamEntities = %v, want the live [Examples.Live]", got) + } + mfs, err := microflowsByQualifiedName(ctx) + if err != nil || mfs["examples.test"] == nil || mfs["examples.test"].Excluded { + t.Errorf("microflowsByQualifiedName picked the excluded twin (err %v)", err) + } + }) + } +} + +// The reported script: a call written against the live flow's parameter. +func TestExcludedTwin_CallAgainstLiveFlowPasses(t *testing.T) { + arg := func(name string) []ast.CallArgument { + return []ast.CallArgument{{Name: name, Value: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "test"}}} + } + for _, order := range twinOrders { + t.Run(order.name, func(t *testing.T) { + ctx := newExcludedTwinCtx(t, order) + ok := []ast.MicroflowStatement{&ast.CallMicroflowStmt{MicroflowName: qn("Examples", "Test"), Arguments: arg("param1")}} + if errs := validateFlowBodyReferences(ctx, ok, newScriptContext()); len(errs) != 0 { + t.Fatalf("call with the live flow's parameter refused: %v", errs) + } + // Control: the excluded twin's parameter is not one the app has. + bad := []ast.MicroflowStatement{&ast.CallMicroflowStmt{MicroflowName: qn("Examples", "Test"), Arguments: arg("param2")}} + errs := validateFlowBodyReferences(ctx, bad, newScriptContext()) + if len(errs) != 1 || !strings.Contains(errs[0], `has no parameter "param2" (declared parameters: param1)`) { + t.Fatalf("errors = %v, want the excluded twin's parameter refused against the live one", errs) + } + }) + } +} diff --git a/mdl/executor/validate_show_page_params.go b/mdl/executor/validate_show_page_params.go index a7e05bd5e..09a931855 100644 --- a/mdl/executor/validate_show_page_params.go +++ b/mdl/executor/validate_show_page_params.go @@ -67,17 +67,21 @@ func buildPageParamNames(ctx *ExecContext) map[string][]string { if err != nil { return out } - for _, p := range all { + for _, p := range liveFirst(all, pageExcluded) { if p == nil { continue } + key := strings.ToLower(h.GetQualifiedName(p.ContainerID, p.Name)) + if _, seen := out[key]; seen { + continue // an excluded twin of a live page (#1928) + } names := []string{} for _, param := range p.Parameters { if param != nil { names = append(names, param.Name) } } - out[strings.ToLower(h.GetQualifiedName(p.ContainerID, p.Name))] = names + out[key] = names } return out } diff --git a/mdl/executor/validate_workflow_refs.go b/mdl/executor/validate_workflow_refs.go index 23b4ab68e..2ecb6b394 100644 --- a/mdl/executor/validate_workflow_refs.go +++ b/mdl/executor/validate_workflow_refs.go @@ -32,8 +32,11 @@ func validateWorkflowParameterMappings(ctx *ExecContext, s *ast.CreateWorkflowSt return nil } paramsByMF := make(map[string][]*microflows.MicroflowParameter, len(mfs)) - for _, mf := range mfs { - paramsByMF[h.GetQualifiedName(mf.ContainerID, mf.Name)] = mf.Parameters + for _, mf := range liveFirst(mfs, microflowExcluded) { + key := h.GetQualifiedName(mf.ContainerID, mf.Name) + if _, seen := paramsByMF[key]; !seen { // live before an excluded twin (#1928) + paramsByMF[key] = mf.Parameters + } } contextEntity := s.ParameterEntity.String() diff --git a/mdl/executor/validate_workflow_task_signature.go b/mdl/executor/validate_workflow_task_signature.go index ade66b56c..1f653c397 100644 --- a/mdl/executor/validate_workflow_task_signature.go +++ b/mdl/executor/validate_workflow_task_signature.go @@ -271,17 +271,21 @@ func buildPageParamEntities(ctx *ExecContext) map[string][]string { if err != nil { return out } - for _, p := range all { + for _, p := range liveFirst(all, pageExcluded) { if p == nil { continue } + key := strings.ToLower(h.GetQualifiedName(p.ContainerID, p.Name)) + if _, seen := out[key]; seen { + continue // an excluded twin of a live page (#1928) + } entities := make([]string, 0, len(p.Parameters)) for _, param := range p.Parameters { if param != nil { entities = append(entities, param.EntityName) } } - out[strings.ToLower(h.GetQualifiedName(p.ContainerID, p.Name))] = entities + out[key] = entities } return out } From 6a9bdafadd9f0c45087e757f476005f528b5648d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 21:13:56 +0000 Subject: [PATCH 4/8] fix(check): count reference errors in the closing summary (#1928) A run that failed on a reference error ended with "0 issues: 0 errors", because the reference tier printed its errors and warnings itself and the summary counts what the other tiers pass to report. They now join the total. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011oK82tQYYsMeM1r2ihkNch --- ...-check-summary-omits-reference-errors.json | 1 + cmd/mxcli/check_reference_summary_test.go | 49 +++++++++++++++++++ cmd/mxcli/cmd_check.go | 13 +++++ 3 files changed, 63 insertions(+) create mode 100644 .claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-check-summary-omits-reference-errors.json create mode 100644 cmd/mxcli/check_reference_summary_test.go diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-check-summary-omits-reference-errors.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-check-summary-omits-reference-errors.json new file mode 100644 index 000000000..6bf4749c5 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-04-check-summary-omits-reference-errors.json @@ -0,0 +1 @@ +{"area":"cmd/mxcli","date":"2026-11-04","symptom":"`mxcli check --references` printed `✗ 1 reference error(s) found` and then `0 issues: 0 errors, 0 warnings, 0 info` — the reporter asked why a reference error does not count as an error (mendixlabs/mxcli#1726)","cause":"the closing summary sums the `printed` list, which only the `report` helper appends to; the reference tier prints its errors and warnings itself, so they never reached the total","file":"cmd/mxcli/cmd_check.go (reference tier appends to printed); test cmd/mxcli/check_reference_summary_test.go","insight":"When one summary is computed over a list that several tiers feed, a tier with its own print path drops out of the total silently. Only the run in the report had an earlier tier's violation, so the summary line printed at all; a script with nothing but a reference error printed no summary, which is why the test asserts `1 errors` rather than the absence of `0 errors`. Control: without the append, the test fails with no summary in the output.","refs":["ako/mxcli#1928","mendixlabs/mxcli#1726"]} diff --git a/cmd/mxcli/check_reference_summary_test.go b/cmd/mxcli/check_reference_summary_test.go new file mode 100644 index 000000000..f81aeb7ee --- /dev/null +++ b/cmd/mxcli/check_reference_summary_test.go @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// ako/mxcli#1928 (mendixlabs/mxcli#1726): a run that failed on a reference +// error ended with "0 issues: 0 errors, 0 warnings, 0 info" — the summary +// counted the violations the other tiers report, and the reference tier +// printed its errors itself. The summary is the total, so it counts them. +func TestCheck_SummaryCountsReferenceErrors(t *testing.T) { + src := filepath.Join("..", "..", "testdata", "pedapp") + if _, err := os.Stat(filepath.Join(src, "PedApp.mpr")); err != nil { + t.Skipf("PedApp fixture not found: %v", err) + } + dir := t.TempDir() + if err := copyTree(src, dir); err != nil { + t.Fatal(err) + } + _ = checkCmd.InheritedFlags() + _ = rootCmd.PersistentFlags().Set("project", filepath.Join(dir, "PedApp.mpr")) + defer func() { + _ = rootCmd.PersistentFlags().Set("project", "") + rootCmd.PersistentFlags().Lookup("project").Changed = false + }() + if f := checkCmd.Flags().Lookup("format"); f != nil { + _ = f.Value.Set(f.DefValue) + } + scripts := t.TempDir() + bad := writeScript(t, scripts, "bad.mdl", `mdl 1; +create or modify microflow MyFirstModule.CallsNothing () +begin + call microflow MyFirstModule.NoSuchFlow1928(); +end; +`) + var code int + out := captureStd(t, func() { code = runCheckFiles(checkCmd, []string{bad}) }) + if code == 0 || !strings.Contains(out, "microflow not found: MyFirstModule.NoSuchFlow1928") { + t.Fatalf("the reference error was not reported (exit %d):\n%s", code, out) + } + if strings.Contains(out, " 0 errors") || !strings.Contains(out, "1 errors") { + t.Errorf("the summary does not count the reference error:\n%s", out) + } +} diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index d5584d090..88e3be321 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -409,6 +409,9 @@ func runCheckFileAgainst(cmd *cobra.Command, filePath, against string) int { for _, w := range refWarnings { fmt.Fprintf(os.Stderr, " %s\n", w) } + // Printed here, not through report, but part of the total the + // closing summary gives (ako/mxcli#1928). + printed = append(printed, warnViolations...) } else if len(warnViolations) > 0 && len(validationErrors) == 0 { structured = append(structured, warnViolations...) } @@ -430,6 +433,16 @@ func runCheckFileAgainst(cmd *cobra.Command, filePath, against string) int { fmt.Fprintf(os.Stderr, " %v\n", err) } fmt.Fprintf(os.Stderr, "\n✗ %d reference error(s) found\n", len(validationErrors)) + // The closing summary is the run's total; it read "0 errors" + // after this line, because these are printed here rather than + // through report (ako/mxcli#1928). + for _, err := range validationErrors { + printed = append(printed, linter.Violation{ + RuleID: "MDL-REF", + Severity: linter.SeverityError, + Message: err.Error(), + }) + } if hint := testFileMissingTargetHint(filePath, validationErrors); hint != "" { fmt.Fprint(os.Stderr, hint) } From 5b4a13943a95e7cbdebb42d0a60fecaa014d1465 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 21:18:06 +0000 Subject: [PATCH 5/8] fix(check): show page sees parameters an earlier alter page adds or drops (#1940) A show page passing a parameter that an earlier alter page ... add parameters in the same script declared was refused by check --references and by exec's pre-flight, because the argument check read the page as stored. The pre-pass now records each alter page's add/drop parameter operations and applies them over the stored or script-declared list. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011oK82tQYYsMeM1r2ihkNch --- ...ow-page-ignores-alter-page-parameters.json | 1 + ...how-page-sees-parameter-added-by-alter.mdl | 15 ++++ mdl/executor/validate.go | 15 +++- mdl/executor/validate_show_page_params.go | 36 ++++++++ .../validate_show_page_params_alter_test.go | 89 +++++++++++++++++++ 5 files changed, 153 insertions(+), 3 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-11-04-show-page-ignores-alter-page-parameters.json create mode 100644 mdl-examples/bug-tests/1940-show-page-sees-parameter-added-by-alter.mdl create mode 100644 mdl/executor/validate_show_page_params_alter_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-show-page-ignores-alter-page-parameters.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-show-page-ignores-alter-page-parameters.json new file mode 100644 index 000000000..36bd2d5e9 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-show-page-ignores-alter-page-parameters.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-04","symptom":"`alter page M.P { add parameters $X: … }` then a show page passing X, in one script: check --references and exec refused it with `page M.P has no parameter \"X\"`; the same statements as two scripts ran (mendixlabs/mxcli#1736)","cause":"the script pre-pass recorded page parameter names only from create page, so for an altered page the show page argument check fell through to the stored parameter list","file":"mdl/executor/validate.go (collectDefinitions AlterPageStmt case, pageParamEdits), mdl/executor/validate_show_page_params.go (recordPageParamEdits, applyPageParamEdits); test validate_show_page_params_alter_test.go","insight":"A script-context index that only listens to create statements reads every alter as a no-op — the same class as the #1355 script-set fix, one level down. Record the alter's edits rather than a computed list, because the pre-pass has no project to read the stored list from; apply them over whichever list wins at lookup time. End-to-end on PedApp 11.13.0: the base build refuses with the reported message, the fix passes check and exec writes both. Control: without the pre-pass case, the add/drop/create-then-alter subtests fail.","refs":["ako/mxcli#1940","mendixlabs/mxcli#1736"]} diff --git a/mdl-examples/bug-tests/1940-show-page-sees-parameter-added-by-alter.mdl b/mdl-examples/bug-tests/1940-show-page-sees-parameter-added-by-alter.mdl new file mode 100644 index 000000000..614d7f830 --- /dev/null +++ b/mdl-examples/bug-tests/1940-show-page-sees-parameter-added-by-alter.mdl @@ -0,0 +1,15 @@ +-- ako/mxcli#1940 (mendixlabs/mxcli#1736): a parameter added by an earlier +-- `alter page … add parameters` in the same script was not declared for a later +-- show page, so check --references and exec refused the script. Run against a +-- project with a page BugTest1940.Edit taking $Account: check passes, exec +-- alters the page and creates the flow. A `drop parameters` earlier in the +-- script likewise removes the name. +mdl 1; +alter page BugTest1940.Edit { + add parameters $Extra: Administration.Account; +}; +create microflow BugTest1940.OpenEdit ($Acc: Administration.Account) +begin + show page BugTest1940.Edit(Account = $Acc, Extra = $Acc); + return; +end; diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index a50eeadd9..d2cf1bad3 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -99,9 +99,13 @@ type scriptContext struct { // the workflow task signature checks, for the same reason as flowParams: the // task page, the targeting microflow, the context entity and the workflow are // ordinarily one script. - pageParams map[string][]string // Module.Page (lower-cased) -> parameter entities - pageParamNames map[string][]string // Module.Page (lower-cased) -> parameter names, for show page arguments - entityGeneralizations map[string]string // Module.Entity (lower-cased) -> generalization, "" for none + pageParams map[string][]string // Module.Page (lower-cased) -> parameter entities + pageParamNames map[string][]string // Module.Page (lower-cased) -> parameter names, for show page arguments + // pageParamEdits is each page's add/drop parameter operations from alter + // page statements, in script order, applied over pageParamNames or the + // stored list (ako/mxcli#1940). + pageParamEdits map[string][]ast.AlterPageOperation + entityGeneralizations map[string]string // Module.Entity (lower-cased) -> generalization, "" for none // Associations and entity attributes declared in the script, for // MDL-XPATH01. Same reason as flowParams above: the overwhelmingly common @@ -173,6 +177,7 @@ func newScriptContext() *scriptContext { pageParams: make(map[string][]string), pageParamNames: make(map[string][]string), + pageParamEdits: make(map[string][]ast.AlterPageOperation), entityGeneralizations: make(map[string]string), } } @@ -336,6 +341,10 @@ func (sc *scriptContext) collectSingle(stmt ast.Statement) { if s.Name.Module != "" { sc.snippets[s.Name.String()] = true } + case *ast.AlterPageStmt: + if s.ContainerType == "PAGE" && s.PageName.Module != "" { + sc.recordPageParamEdits(s.PageName.String(), s.Operations) + } case *ast.CreateLayoutStmt: if s.Name.Module != "" { sc.layouts[s.Name.String()] = true diff --git a/mdl/executor/validate_show_page_params.go b/mdl/executor/validate_show_page_params.go index 09a931855..9fa2920b8 100644 --- a/mdl/executor/validate_show_page_params.go +++ b/mdl/executor/validate_show_page_params.go @@ -3,6 +3,7 @@ package executor import ( + "slices" "strings" "github.com/mendixlabs/mxcli/mdl/ast" @@ -30,6 +31,7 @@ func validateShowPageParams(ctx *ExecContext, calls []codeActionCallRef, sc *scr if !ok { continue } + declared = applyPageParamEdits(declared, sc.pageParamEdits[key]) for _, msg := range validateCodeActionParams("page", call, declared) { for _, written := range call.argNames { if strings.EqualFold(written, "NumberOfPagesToClose") && strings.Contains(msg, `"`+written+`"`) { @@ -96,3 +98,37 @@ func (sc *scriptContext) recordPageParamNames(qualifiedName string, params []ast } sc.pageParamNames[strings.ToLower(qualifiedName)] = names } + +// recordPageParamEdits keeps an alter page's add/drop parameter operations, so +// a show page later in the script is checked against the page as the script +// leaves it, not as it is stored (ako/mxcli#1940): adding a parameter to a page +// and passing it from the flow that opens the page is one change. +func (sc *scriptContext) recordPageParamEdits(qualifiedName string, ops []ast.AlterPageOperation) { + key := strings.ToLower(qualifiedName) + for _, op := range ops { + switch op.(type) { + case *ast.AddParameterOp, *ast.DropParameterOp: + sc.pageParamEdits[key] = append(sc.pageParamEdits[key], op) + } + } +} + +// applyPageParamEdits returns declared with the edits applied in order. It +// never changes declared, which may be the stored page's list. +func applyPageParamEdits(declared []string, edits []ast.AlterPageOperation) []string { + if len(edits) == 0 { + return declared + } + out := append([]string{}, declared...) + for _, op := range edits { + switch o := op.(type) { + case *ast.AddParameterOp: + if !slices.Contains(out, o.Parameter.Name) { + out = append(out, o.Parameter.Name) + } + case *ast.DropParameterOp: + out = slices.DeleteFunc(out, func(n string) bool { return n == o.ParameterName }) + } + } + return out +} diff --git a/mdl/executor/validate_show_page_params_alter_test.go b/mdl/executor/validate_show_page_params_alter_test.go new file mode 100644 index 000000000..939a0aa08 --- /dev/null +++ b/mdl/executor/validate_show_page_params_alter_test.go @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1940 (mendixlabs/mxcli#1736): `alter page … { add parameters $X }` +// followed in the same script by a show page passing X was refused — by check +// --references and by exec's pre-flight — because the argument check read the +// page as stored before the script. Split into two scripts, both ran. +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +func TestValidate_ShowPageSeesParametersAlteredInTheScript(t *testing.T) { + mod := &model.Module{Name: "M"} + mod.ID = "mod1" + stored := &pages.Page{Name: "Edit", Parameters: []*pages.PageParameter{{Name: "District"}, {Name: "Choice"}}} + stored.ContainerID = "mod1" + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListPagesFunc: func() ([]*pages.Page, error) { return []*pages.Page{stored}, nil }, + } + show := func(page string, names ...string) ast.MicroflowStatement { + var args []ast.ShowPageArg + for _, n := range names { + args = append(args, ast.ShowPageArg{ParamName: n, Value: &ast.VariableExpr{Name: "V"}}) + } + return &ast.ShowPageStmt{PageName: qn("M", page), Arguments: args} + } + alter := func(page string, ops ...ast.AlterPageOperation) ast.Statement { + return &ast.AlterPageStmt{ContainerType: "PAGE", PageName: qn("M", page), Operations: ops} + } + add := func(name string) ast.AlterPageOperation { + return &ast.AddParameterOp{Parameter: ast.PageParameter{Name: name}} + } + drop := func(name string) ast.AlterPageOperation { return &ast.DropParameterOp{ParameterName: name} } + + cases := []struct { + name string + before []ast.Statement + call ast.MicroflowStatement + wantErr string + }{ + {"the reported script: a parameter added to a stored page", + []ast.Statement{alter("Edit", add("Extra"))}, show("Edit", "District", "Choice", "Extra"), ""}, + {"a parameter dropped from a stored page is no longer declared", + []ast.Statement{alter("Edit", drop("Choice"))}, show("Edit", "District", "Choice"), + `page M.Edit has no parameter "Choice" (declared parameters: District)`}, + {"add then drop across two alters", + []ast.Statement{alter("Edit", add("Extra")), alter("Edit", drop("Extra"))}, show("Edit", "Extra"), + `page M.Edit has no parameter "Extra" (declared parameters: Choice, District)`}, + {"a page created in the script, then altered", + []ast.Statement{&ast.CreatePageStmtV3{Name: qn("M", "New")}, alter("New", add("Obj"))}, show("New", "Obj"), ""}, + {"control: no alter, the stored page still refuses an unknown name", + nil, show("Edit", "Extra"), `page M.Edit has no parameter "Extra" (declared parameters: Choice, District)`}, + {"control: an alter snippet does not change the page of that name", + []ast.Statement{&ast.AlterPageStmt{ContainerType: "SNIPPET", PageName: qn("M", "Edit"), Operations: []ast.AlterPageOperation{add("Extra")}}}, + show("Edit", "Extra"), `page M.Edit has no parameter "Extra"`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(mb)) + body := []ast.MicroflowStatement{tc.call} + stmts := append(append([]ast.Statement{}, tc.before...), &ast.CreateMicroflowStmt{Name: qn("M", "Caller"), Body: body}) + sc := newScriptContext() + sc.collectDefinitions(&ast.Program{Statements: stmts}) + errs := validateFlowBodyReferences(ctx, body, sc) + if tc.wantErr == "" { + if len(errs) != 0 { + t.Fatalf("refused: %v", errs) + } + return + } + if len(errs) != 1 || !strings.Contains(errs[0], tc.wantErr) { + t.Fatalf("errors = %v, want one containing %q", errs, tc.wantErr) + } + }) + } + // The stored page itself is untouched by the simulation. + if len(stored.Parameters) != 2 { + t.Errorf("stored page parameters changed: %d", len(stored.Parameters)) + } +} From ecfa2170b6a22e530945eac2465a41cf68c99fe1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 21:26:49 +0000 Subject: [PATCH 6/8] fix(check): resolve the target of drop, rename, move and alter flow/enumeration (#1821) check --references resolved the target document of alter entity and alter page only; drop, rename and move of a document that does not exist, and alter enumeration / alter microflow / alter nanoflow on one, passed check and were refused by exec. Every form now resolves its target against the same listings, reporting the near names the module has. A name an earlier statement provides is not refused: drop if exists, a create, rename or move earlier in the script, a renamed module, and modules an execute script, generated connector, business event service or external-entities import fills without naming. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011oK82tQYYsMeM1r2ihkNch --- ...check-resolves-every-statement-target.json | 1 + cmd/mxcli/check_statement_target_test.go | 108 +++++++++++++ .../1821-check-resolves-statement-targets.mdl | 19 +++ mdl/executor/validate.go | 45 ++++-- mdl/executor/validate_alter_target.go | 146 +++++++++++++++--- 5 files changed, 287 insertions(+), 32 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-every-statement-target.json create mode 100644 cmd/mxcli/check_statement_target_test.go create mode 100644 mdl-examples/bug-tests/1821-check-resolves-statement-targets.mdl diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-every-statement-target.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-every-statement-target.json new file mode 100644 index 000000000..0a1bb0de6 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-every-statement-target.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-04","symptom":"`check --references` passed drop/rename/move of a document that does not exist, and alter enumeration / alter microflow on one, while exec refused each with ` not found` — 9 of the 11 statement forms that name an existing document (only alter entity and alter page were resolved)","cause":"validateAlterTarget was written for ALTER and extended one statement at a time; the drop/rename/move statements resolve against the same listings but were never routed to it","file":"mdl/executor/validate_alter_target.go (validateAlterTarget, targetListing, checkTarget, moveTargetKind), mdl/executor/validate.go (collectSingle: indirectDocs, renamedModules, opaqueModules, opaqueScript); test cmd/mxcli/check_statement_target_test.go","insight":"The risk in widening a not-found check is the false refusal, not the miss: a document can reach a name without a create statement naming it — an earlier rename or move, a rename module, an execute script, a generated SQL connector, a business event service, a bulk external-entities import. Record each in the pre-pass and stay silent for that name or module; the check must stay quieter than exec. Matching is exact-case because exec's lookups are. A PedApp test per form, each with an existing-target control, plus if-exists / created / renamed / moved controls. Control: without the new cases all 13 forms pass check.","refs":["ako/mxcli#1821","ako/mxcli#555"]} diff --git a/cmd/mxcli/check_statement_target_test.go b/cmd/mxcli/check_statement_target_test.go new file mode 100644 index 000000000..104d75e03 --- /dev/null +++ b/cmd/mxcli/check_statement_target_test.go @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// ako/mxcli#1821: `check --references` resolved the target document of alter +// entity and alter page only. drop, rename and move of a document that does not +// exist — and alter enumeration / alter microflow — passed check and were then +// refused by exec. Every form is checked against PedApp: a missing target is +// reported, and the same statement on a target the project has is not. +func TestCheck_StatementTargetMustExist(t *testing.T) { + src := filepath.Join("..", "..", "testdata", "pedapp") + if _, err := os.Stat(filepath.Join(src, "PedApp.mpr")); err != nil { + t.Skipf("PedApp fixture not found: %v", err) + } + dir := t.TempDir() + if err := copyTree(src, dir); err != nil { + t.Fatal(err) + } + _ = checkCmd.InheritedFlags() + _ = rootCmd.PersistentFlags().Set("project", filepath.Join(dir, "PedApp.mpr")) + defer func() { + _ = rootCmd.PersistentFlags().Set("project", "") + rootCmd.PersistentFlags().Lookup("project").Changed = false + }() + if f := checkCmd.Flags().Lookup("format"); f != nil { + _ = f.Value.Set(f.DefValue) + } + scripts := t.TempDir() + run := func(name, body string) (int, string) { + path := writeScript(t, scripts, name+".mdl", "mdl 1;\n"+body+"\n") + var code int + out := captureStd(t, func() { code = runCheckFiles(checkCmd, []string{path}) }) + return code, out + } + + cases := []struct { + name, missing, want, existing string + }{ + {"drop microflow", "drop microflow MyFirstModule.NoSuchFlow;", "microflow not found: MyFirstModule.NoSuchFlow", + "drop microflow MyFirstModule.MyFirstLogic;"}, + {"drop nanoflow", "drop nanoflow Atlas_Web_Content.NoSuchNano;", "nanoflow not found: Atlas_Web_Content.NoSuchNano", + "drop nanoflow Atlas_Web_Content.ACT_Login;"}, + {"drop page", "drop page MyFirstModule.NoSuchPage;", "page not found: MyFirstModule.NoSuchPage", + "drop page MyFirstModule.Home_Web;"}, + {"drop snippet", "drop snippet Administration.NoSuchSnippet;", "snippet not found: Administration.NoSuchSnippet", + "drop snippet Administration.ReadMe;"}, + {"drop entity", "drop entity Administration.NoSuchEntity;", "entity not found: Administration.NoSuchEntity", + "drop entity Administration.AccountPasswordData;"}, + {"drop enumeration", "drop enumeration FeedbackModule.NoSuchEnum;", "enumeration not found: FeedbackModule.NoSuchEnum", + "drop enumeration FeedbackModule.LogNodes;"}, + {"drop association", "drop association Administration.NoSuchAssoc;", "association not found: Administration.NoSuchAssoc", + "drop association Administration.AccountPasswordData_Account;"}, + {"drop constant", "drop constant FeedbackModule.NoSuchConst;", "constant not found: FeedbackModule.NoSuchConst", + "drop constant FeedbackModule.LocalStorageKey;"}, + {"rename microflow", "rename microflow MyFirstModule.NoSuchFlow to Other;", "microflow not found: MyFirstModule.NoSuchFlow", + "rename microflow MyFirstModule.MyFirstLogic to MyRenamedLogic;"}, + {"move microflow", "move microflow MyFirstModule.NoSuchFlow to folder 'X';", "microflow not found: MyFirstModule.NoSuchFlow", + "move microflow MyFirstModule.MyFirstLogic to folder 'X';"}, + {"alter enumeration", "alter enumeration FeedbackModule.NoSuchEnum add value V caption 'V';", "enumeration not found: FeedbackModule.NoSuchEnum", + "alter enumeration FeedbackModule.LogNodes add value if not exists V1821 caption 'V';"}, + {"alter microflow", "alter microflow MyFirstModule.NoSuchFlow { drop $X; };", "microflow not found: MyFirstModule.NoSuchFlow", + ""}, + {"alter nanoflow", "alter nanoflow Atlas_Web_Content.NoSuchNano { drop $X; };", "nanoflow not found: Atlas_Web_Content.NoSuchNano", + ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + code, out := run(strings.ReplaceAll(tc.name, " ", "_"), tc.missing) + if code == 0 || !strings.Contains(out, tc.want) { + t.Errorf("a missing target passed check (exit %d), want %q:\n%s", code, tc.want, out) + } + if tc.existing == "" { + return + } + // Control: the same form on a target the project has is not "not found". + _, out = run(strings.ReplaceAll(tc.name, " ", "_")+"_ok", tc.existing) + if strings.Contains(out, "not found") { + t.Errorf("control: an existing target was reported:\n%s", out) + } + }) + } + + // Controls the check must not break: drop … if exists, a document the + // script creates first, and one an earlier rename lands under the name. + for name, body := range map[string]string{ + "if exists": "drop microflow if exists MyFirstModule.NoSuchFlow;", + "created in the script": `create microflow MyFirstModule.Temp1821 () begin end; +drop microflow MyFirstModule.Temp1821;`, + "renamed earlier in the script": `rename microflow MyFirstModule.MyFirstLogic to Renamed1821; +drop microflow MyFirstModule.Renamed1821;`, + "moved earlier in the script": `move page MyFirstModule.Home_Web to Administration; +drop page Administration.Home_Web;`, + } { + t.Run("control: "+name, func(t *testing.T) { + _, out := run("control_"+strings.ReplaceAll(name, " ", "_"), body) + if strings.Contains(out, "not found") { + t.Errorf("reported a target the script provides:\n%s", out) + } + }) + } +} diff --git a/mdl-examples/bug-tests/1821-check-resolves-statement-targets.mdl b/mdl-examples/bug-tests/1821-check-resolves-statement-targets.mdl new file mode 100644 index 000000000..fd915139a --- /dev/null +++ b/mdl-examples/bug-tests/1821-check-resolves-statement-targets.mdl @@ -0,0 +1,19 @@ +-- ako/mxcli#1821: `check --references` resolved the target document of alter +-- entity and alter page only. Each statement below names a document that does +-- not exist; exec refuses every one ("microflow not found", …), and check now +-- reports each in the alter entity style, with what the module does have: +-- microflow not found: BugTest1821.NoSuchFlow (referenced by drop microflow) — BugTest1821 has … +-- Run against a project with a BugTest1821 module: every statement is reported. +-- `drop … if exists`, a document the script creates first, and one an earlier +-- rename or move lands under the name are not reported. +mdl 1; +drop microflow BugTest1821.NoSuchFlow; +drop page BugTest1821.NoSuchPage; +drop entity BugTest1821.NoSuchEntity; +drop enumeration BugTest1821.NoSuchEnum; +drop association BugTest1821.NoSuchAssoc; +rename microflow BugTest1821.NoSuchFlow to Other; +move microflow BugTest1821.NoSuchFlow to folder 'X'; +alter enumeration BugTest1821.NoSuchEnum add value V caption 'V'; +alter microflow BugTest1821.NoSuchFlow { drop $X; }; +drop microflow if exists BugTest1821.NoSuchFlow; diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index d2cf1bad3..415f74e2b 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -36,15 +36,25 @@ type scriptContext struct { // association endpoint check from refusing what exec will find (#555). indirectEntities map[string]bool externalEntityModules map[string]bool - enumerations map[string]bool // Enumerations created (Module.Enum) - microflows map[string]bool // Microflows created (Module.Microflow) - nanoflows map[string]bool // Nanoflows created (Module.Nanoflow) - pages map[string]bool // Pages created (Module.Page) - snippets map[string]bool // Snippets created (Module.Snippet) - layouts map[string]bool // Layouts created (Module.Layout) - menus map[string]bool // Menu documents created (Module.Menu) - constants map[string]bool // Constants created (Module.Constant) - workflows map[string]bool // Workflows created (Module.Workflow) + // indirectDocs is the same for every document kind ("microflow:M.Name"), + // and renamedModules the names a rename module gives, so the statement + // target check does not refuse what an earlier statement provides + // (ako/mxcli#1821). opaqueModules are modules a statement fills with + // documents it does not name (generated connectors, business event + // services); opaqueScript is an execute script, which can create anything. + indirectDocs map[string]bool + renamedModules map[string]bool + opaqueModules map[string]bool + opaqueScript bool + enumerations map[string]bool // Enumerations created (Module.Enum) + microflows map[string]bool // Microflows created (Module.Microflow) + nanoflows map[string]bool // Nanoflows created (Module.Nanoflow) + pages map[string]bool // Pages created (Module.Page) + snippets map[string]bool // Snippets created (Module.Snippet) + layouts map[string]bool // Layouts created (Module.Layout) + menus map[string]bool // Menu documents created (Module.Menu) + constants map[string]bool // Constants created (Module.Constant) + workflows map[string]bool // Workflows created (Module.Workflow) // Task queues created (Module.Queue, lower-cased — a queue is resolved // case-insensitively, like buildQueueQualifiedNames). Missing, a queue the // script creates was "not found" for every `in queue` call to it @@ -178,6 +188,9 @@ func newScriptContext() *scriptContext { pageParams: make(map[string][]string), pageParamNames: make(map[string][]string), pageParamEdits: make(map[string][]ast.AlterPageOperation), + indirectDocs: make(map[string]bool), + renamedModules: make(map[string]bool), + opaqueModules: make(map[string]bool), entityGeneralizations: make(map[string]string), } } @@ -309,10 +322,24 @@ func (sc *scriptContext) collectSingle(stmt ast.Statement) { if s.ObjectType == "entity" && s.Name.Module != "" && s.NewName != "" { sc.indirectEntities[s.Name.Module+"."+s.NewName] = true } + if s.ObjectType == "module" && s.NewName != "" { + sc.renamedModules[s.NewName] = true + } else if s.Name.Module != "" && s.NewName != "" && !s.DryRun { + sc.indirectDocs[s.ObjectType+":"+s.Name.Module+"."+s.NewName] = true + } case *ast.MoveStmt: if s.DocumentType == ast.DocumentTypeEntity && s.TargetModule != "" { sc.indirectEntities[s.TargetModule+"."+s.Name.Name] = true } + if s.TargetModule != "" { + sc.indirectDocs[moveTargetKind(s.DocumentType)+":"+s.TargetModule+"."+s.Name.Name] = true + } + case *ast.ExecuteScriptStmt: + sc.opaqueScript = true + case *ast.SQLGenerateConnectorStmt: + sc.opaqueModules[s.Module] = true + case *ast.CreateBusinessEventServiceStmt: + sc.opaqueModules[s.Name.Module] = true case *ast.CreateEnumerationStmt: if s.Name.Module != "" { sc.enumerations[s.Name.String()] = true diff --git a/mdl/executor/validate_alter_target.go b/mdl/executor/validate_alter_target.go index a39168b96..f0f9d6ae6 100644 --- a/mdl/executor/validate_alter_target.go +++ b/mdl/executor/validate_alter_target.go @@ -33,61 +33,161 @@ import ( // a false "not found" blocks a script that would have worked, which is worse // than the silence it replaces. -// validateAlterTarget reports an ALTER whose target document does not exist, -// counting documents the script itself creates earlier. +// validateAlterTarget reports a statement whose target document does not +// exist, counting documents the script itself creates or lands under the name. +// +// It began with ALTER; drop, rename and move name an existing document the same +// way and passed check while exec refused them — 9 of the 11 forms that name +// one (ako/mxcli#1821). They resolve against the same listings, so each kind +// below means the same thing for every verb. func validateAlterTarget(ctx *ExecContext, stmt ast.Statement, sc *scriptContext) error { if !ctx.Connected() { return nil } + // DROP … IF EXISTS names something that may legitimately be absent. + if g, ok := stmt.(ast.IfExistsDrop); ok && g.DropIfExists() { + return nil + } switch s := stmt.(type) { case *ast.AlterPageStmt: // One statement type, three document kinds — the visitor sets // ContainerType from the keyword. switch strings.ToUpper(s.ContainerType) { case "SNIPPET": - return checkAlterTarget(ctx, sc, "snippet", s.PageName, - buildSnippetQualifiedNames, func(qn string) bool { return sc.snippets[qn] }) + return checkTarget(ctx, sc, "alter", "snippet", s.PageName) case "LAYOUT": - return checkAlterTarget(ctx, sc, "layout", s.PageName, - buildLayoutQualifiedNames, func(qn string) bool { return sc.layouts[qn] }) + return checkTarget(ctx, sc, "alter", "layout", s.PageName) default: - return checkAlterTarget(ctx, sc, "page", s.PageName, - buildPageQualifiedNames, func(qn string) bool { return sc.pages[qn] }) + return checkTarget(ctx, sc, "alter", "page", s.PageName) } case *ast.AlterEntityStmt: + return checkTarget(ctx, sc, "alter", "entity", s.Name) + case *ast.AlterEnumerationStmt: + return checkTarget(ctx, sc, "alter", "enumeration", s.Name) + case *ast.AlterFlowStmt: + return checkTarget(ctx, sc, "alter", s.Kind(), s.Name) + case *ast.DropMicroflowStmt: + return checkTarget(ctx, sc, "drop", "microflow", s.Name) + case *ast.DropNanoflowStmt: + return checkTarget(ctx, sc, "drop", "nanoflow", s.Name) + case *ast.DropPageStmt: + return checkTarget(ctx, sc, "drop", "page", s.Name) + case *ast.DropSnippetStmt: + return checkTarget(ctx, sc, "drop", "snippet", s.Name) + case *ast.DropEntityStmt: + return checkTarget(ctx, sc, "drop", "entity", s.Name) + case *ast.DropEnumerationStmt: + return checkTarget(ctx, sc, "drop", "enumeration", s.Name) + case *ast.DropAssociationStmt: + return checkTarget(ctx, sc, "drop", "association", s.Name) + case *ast.DropConstantStmt: + return checkTarget(ctx, sc, "drop", "constant", s.Name) + case *ast.RenameStmt: + if s.DryRun { + return nil + } + return checkTarget(ctx, sc, "rename", s.ObjectType, s.Name) + case *ast.MoveStmt: + return checkTarget(ctx, sc, "move", moveTargetKind(s.DocumentType), s.Name) + } + return nil +} + +// moveTargetKind is a move's document type as the kind name rename and the +// target check use; "" for one they do not resolve. +func moveTargetKind(t ast.DocumentType) string { + switch t { + case ast.DocumentTypePage: + return "page" + case ast.DocumentTypeMicroflow: + return "microflow" + case ast.DocumentTypeNanoflow: + return "nanoflow" + case ast.DocumentTypeSnippet: + return "snippet" + case ast.DocumentTypeEntity: + return "entity" + case ast.DocumentTypeEnumeration: + return "enumeration" + case ast.DocumentTypeConstant: + return "constant" + } + return "" +} + +// targetListing is, per kind, the project's documents and the script's own. +// A kind missing here is not resolved: a false "not found" blocks a script +// that would have run, which is worse than the silence it replaces. +func targetListing(ctx *ExecContext, sc *scriptContext, kind string) (stored func() map[string]bool, inScript func(string) bool) { + switch kind { + case "page": + return func() map[string]bool { return buildPageQualifiedNames(ctx) }, func(qn string) bool { return sc.pages[qn] } + case "snippet": + return func() map[string]bool { return buildSnippetQualifiedNames(ctx) }, func(qn string) bool { return sc.snippets[qn] } + case "layout": + return func() map[string]bool { return buildLayoutQualifiedNames(ctx) }, func(qn string) bool { return sc.layouts[qn] } + case "microflow": + return func() map[string]bool { return buildMicroflowQualifiedNames(ctx) }, func(qn string) bool { return sc.microflows[qn] } + case "nanoflow": + return func() map[string]bool { return buildNanoflowQualifiedNames(ctx) }, func(qn string) bool { return sc.nanoflows[qn] } + case "entity": // A MOVE or RENAME earlier in the script lands an entity under a name // no CREATE declares (mendixlabs/mxcli#1601). - return checkAlterTarget(ctx, sc, "entity", s.Name, buildEntityQualifiedNames, - func(qn string) bool { return sc.entities[qn] || sc.indirectEntities[qn] }) + return func() map[string]bool { return buildEntityQualifiedNames(ctx) }, + func(qn string) bool { return sc.entities[qn] || sc.indirectEntities[qn] } + case "enumeration": + return func() map[string]bool { return buildEnumerationQualifiedNames(ctx) }, func(qn string) bool { return sc.enumerations[qn] } + case "association": + return func() map[string]bool { return buildAssociationQualifiedNames(ctx) }, + func(qn string) bool { + _, name, _ := strings.Cut(qn, ".") + return sc.associations[name] == qn || sc.ambiguousAssc[name] + } + case "constant": + return func() map[string]bool { + names, ok := buildConstantQualifiedNames(ctx) + if !ok { + return nil + } + return names + }, + func(qn string) bool { return sc.constants[qn] } } - return nil + return nil, nil } -// checkAlterTarget resolves one target, and on a miss reports the near names so +// checkTarget resolves one target, and on a miss reports the near names so // the author can see the typo rather than only that they made one. -func checkAlterTarget(ctx *ExecContext, sc *scriptContext, kind string, name ast.QualifiedName, - known func(*ExecContext) map[string]bool, inScript func(string) bool) error { +func checkTarget(ctx *ExecContext, sc *scriptContext, verb, kind string, name ast.QualifiedName) error { + stored, inScript := targetListing(ctx, sc, kind) + if stored == nil { + return nil + } qn := name.String() - // An unqualified name is a different error, already reported elsewhere; and - // a module the script creates has no listing to resolve against yet. - if name.Module == "" || sc.modules[name.Module] { + // An unqualified name is a different error, already reported elsewhere; a + // module the script creates or renames has no listing to resolve against + // yet; and a module something fills without naming its documents cannot + // be judged from the statements. + if name.Module == "" || sc.modules[name.Module] || sc.renamedModules[name.Module] || + sc.opaqueScript || sc.opaqueModules[name.Module] || + (kind == "entity" && sc.externalEntityModules[name.Module]) { return nil } - if inScript(qn) { + if inScript(qn) || sc.indirectDocs[kind+":"+qn] { return nil } - stored := known(ctx) - if stored[qn] { + known := stored() + if known[qn] { return nil } // An empty listing means the backend could not answer, not that the project // has no pages. Reporting "not found" from it would fail every script. - if len(stored) == 0 { + if len(known) == 0 { return nil } return mdlerrors.NewNotFoundMsg(kind, qn, fmt.Sprintf( - "%s not found: %s (referenced by alter %s)%s", kind, qn, kind, - nearNamesIn(stored, name.Module))) + "%s not found: %s (referenced by %s %s)%s", kind, qn, verb, kind, + nearNamesIn(known, name.Module))) } // nearNamesIn lists what the module does have, capped so a large module does not From 89677039e2d013f7869dcbe3005d3c93bb381cf2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 21:26:49 +0000 Subject: [PATCH 7/8] fix(check): refuse a widget binding to an attribute the entity lacks (#555) An input widget's Attribute, or a ContentParams / CaptionParams binding, naming an attribute the data container's entity does not have passed check --references; exec wrote it and mxbuild refused it with CE1613 (3 of 3 on a fresh 11.12.6 app, control page clean). The scope walk now reports it, listing the entity's attributes. Unresolved contexts, altered entities, expressions, $var paths and audit members are not judged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011oK82tQYYsMeM1r2ihkNch --- ...1-04-check-resolves-widget-attributes.json | 1 + .../check_widget_attribute_exists_test.go | 76 ++++++++++++++++ .../555-check-resolves-widget-attributes.mdl | 19 ++++ .../validate_widget_attribute_scope.go | 1 + .../validate_widget_attribute_type.go | 90 +++++++++++++++++++ .../validate_widget_attribute_type_test.go | 8 +- 6 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-widget-attributes.json create mode 100644 cmd/mxcli/check_widget_attribute_exists_test.go create mode 100644 mdl-examples/bug-tests/555-check-resolves-widget-attributes.mdl diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-widget-attributes.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-widget-attributes.json new file mode 100644 index 000000000..dc6c5df30 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-check-resolves-widget-attributes.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-04","symptom":"`textbox (Attribute: PlannedOn)` in a data view, or `ContentParams: ({1} = PlannedOn)` / `CaptionParams`, on an entity without that attribute: check --references passed, exec wrote it, mxbuild CE1613 \"The selected attribute 'M.E.PlannedOn' no longer exists\" — measured 3 of 3 on a fresh 11.12.6 app, control page clean","cause":"the input-widget scope walk (checkInputBinding) looked the attribute up and returned silently when it was not found unless the name was an association; template parameters were never looked up","file":"mdl/executor/validate_widget_attribute_type.go (reportMissingAttribute, checkTemplateParams), mdl/executor/validate_widget_attribute_scope.go (walk); test cmd/mxcli/check_widget_attribute_exists_test.go","insight":"The context walk, the member index with inheritance and the association hops were already there for the type rule (MDL-WIDGET39) — the not-found case was a silent `return` in it. Before writing a new resolver, grep for the lookup that already returns found/known and see what it does on a miss. Stay quiet where the index is not sure: an unknown chain (known=false), an unresolved context, an altered entity, an expression or $var path, and the audit members (createdDate/changedDate/owner/changedBy), which bind by name but are not in the attribute list. Control: without the two calls the textbox and content-parameter subtests pass check.","refs":["ako/mxcli#555"]} diff --git a/cmd/mxcli/check_widget_attribute_exists_test.go b/cmd/mxcli/check_widget_attribute_exists_test.go new file mode 100644 index 000000000..41e41c8b9 --- /dev/null +++ b/cmd/mxcli/check_widget_attribute_exists_test.go @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// ako/mxcli#555: an attribute named inside a widget was not resolved by +// `check --references` — a textbox's Attribute, or a template parameter of a +// dynamic text — so a misspelt name passed check, exec wrote it, and mxbuild +// refused it with CE1613. Each binding is now resolved against the entity its +// data container supplies, and the controls bind what Account has: its own +// attribute, an inherited one, an audit member and an expression. +func TestCheck_WidgetAttributeMustExist(t *testing.T) { + src := filepath.Join("..", "..", "testdata", "pedapp") + if _, err := os.Stat(filepath.Join(src, "PedApp.mpr")); err != nil { + t.Skipf("PedApp fixture not found: %v", err) + } + dir := t.TempDir() + if err := copyTree(src, dir); err != nil { + t.Fatal(err) + } + _ = checkCmd.InheritedFlags() + _ = rootCmd.PersistentFlags().Set("project", filepath.Join(dir, "PedApp.mpr")) + defer func() { + _ = rootCmd.PersistentFlags().Set("project", "") + rootCmd.PersistentFlags().Lookup("project").Changed = false + }() + if f := checkCmd.Flags().Lookup("format"); f != nil { + _ = f.Value.Set(f.DefValue) + } + scripts := t.TempDir() + page := func(file, widgets string) (int, string) { + path := writeScript(t, scripts, file+".mdl", `mdl 1; +create page MyFirstModule.P555 ( Title: 'x', Layout: Atlas_Core.PopupLayout, Params: ( $Acc: Administration.Account ) ) +{ + layoutgrid lg { row { column (desktopwidth: autofill) { + dataview dv (DataSource: $Acc) { +`+widgets+` + } + } } } +}; +`) + var code int + out := captureStd(t, func() { code = runCheckFiles(checkCmd, []string{path}) }) + return code, out + } + + for _, tc := range []struct{ name, widgets, want string }{ + {"textbox attribute", `textbox tb (Label: 'x', Attribute: PlannedOn)`, + "widget `tb` (textbox) binds `PlannedOn`, but Administration.Account has no attribute PlannedOn"}, + {"dynamic text content parameter", `dynamictext dt (Content: 'Hi {1}', ContentParams: ({1} = PlannedOn))`, + "widget `dt` (dynamictext) ContentParams {1} binds `PlannedOn`, but Administration.Account has no attribute PlannedOn"}, + } { + t.Run(tc.name, func(t *testing.T) { + code, out := page(strings.ReplaceAll(tc.name, " ", "_"), tc.widgets) + if code == 0 || !strings.Contains(out, tc.want) || !strings.Contains(out, "CE1613") { + t.Errorf("a missing attribute passed check (exit %d), want %q:\n%s", code, tc.want, out) + } + }) + } + + t.Run("control: attributes the entity has", func(t *testing.T) { + code, out := page("control", `textbox tb1 (Label: 'x', Attribute: FullName) + textbox tb2 (Label: 'x', Attribute: Name) + datepicker dp (Label: 'x', Attribute: changedDate) + dynamictext dt1 (Content: '{1} {2} {3}', ContentParams: ({1} = FullName, {2} = 'literal', {3} = $Acc/Email))`) + if code != 0 || strings.Contains(out, "has no attribute") { + t.Errorf("an attribute Account has was refused (exit %d):\n%s", code, out) + } + }) +} diff --git a/mdl-examples/bug-tests/555-check-resolves-widget-attributes.mdl b/mdl-examples/bug-tests/555-check-resolves-widget-attributes.mdl new file mode 100644 index 000000000..5cebd55ec --- /dev/null +++ b/mdl-examples/bug-tests/555-check-resolves-widget-attributes.mdl @@ -0,0 +1,19 @@ +-- ako/mxcli#555: an attribute named inside a widget was not resolved by +-- `check --references`, so a misspelt name passed check, exec wrote it and +-- mxbuild refused it. Measured on a fresh 11.12.6 app: each of the three +-- bindings below is CE1613 "The selected attribute 'BugTest555.WorkOrder.PlannedOn' +-- no longer exists" (3 errors), and the same page binding PlannedDate / Title +-- builds clean. check -p --references now reports all three before the write. +mdl 1; +create module BugTest555; +create persistent entity BugTest555.WorkOrder ( PlannedDate: DateTime, Title: String(100) ); +create page BugTest555.P_WorkOrder ( Title: 'x', Layout: Atlas_Core.Atlas_Default, Params: ( $W: BugTest555.WorkOrder ) ) +{ + layoutgrid lg { row { column (desktopwidth: autofill) { + dataview dv (DataSource: $W) { + textbox tb (Label: 'Planned', Attribute: PlannedOn) + dynamictext dt (Content: 'Planned {1}', ContentParams: ({1} = PlannedOn)) + actionbutton btn (Caption: 'Go {1}', CaptionParams: ({1} = PlannedOn), Action: close page) + } + } } } +}; diff --git a/mdl/executor/validate_widget_attribute_scope.go b/mdl/executor/validate_widget_attribute_scope.go index f1ffe47e6..83298b017 100644 --- a/mdl/executor/validate_widget_attribute_scope.go +++ b/mdl/executor/validate_widget_attribute_scope.go @@ -121,6 +121,7 @@ func (v *attributeScopeValidator) walk(w *ast.WidgetV3, enclosing dataContext) { } inner := childContext(enclosing, w.GetDataSource(), v.sigs, v.pageParams) v.checkInputBinding(w, enclosing) + v.checkTemplateParams(w, enclosing) v.checkAssocListSource(w, enclosing) v.checkViewAssocReverse(w, enclosing) v.checkAssocHopStart(w, enclosing) diff --git a/mdl/executor/validate_widget_attribute_type.go b/mdl/executor/validate_widget_attribute_type.go index 2ba843c55..7cfca730d 100644 --- a/mdl/executor/validate_widget_attribute_type.go +++ b/mdl/executor/validate_widget_attribute_type.go @@ -386,8 +386,12 @@ func (v *attributeScopeValidator) checkInputBinding(w *ast.WidgetV3, enclosing d "anything else). Bind an attribute over it (`%s/`), or select the associated object "+ "with a `combobox` over the association ["+widgetAttributeTypeRule+"]", w.Name, strings.ToLower(w.Type), name, assocQN, entity, name, name)) + return } } + if !isComboBox { + v.reportMissingAttribute(w, "", attr, entity, name) + } return } if a.kind == "" || !measuredAttributeKinds[a.kind] || rule.accepts[a.kind] { @@ -403,6 +407,92 @@ func (v *attributeScopeValidator) checkInputBinding(w *ast.WidgetV3, enclosing d v.errs = append(v.errs, msg+" ["+widgetAttributeTypeRule+"]") } +// auditMembers are the system members an entity's audit settings add — +// bindable by these names, and never in its attribute list. +var auditMembers = kindSet("createddate", "changeddate", "owner", "changedby") + +// reportMissingAttribute is CE1613 for a binding to an attribute the entity +// in context does not have (ako/mxcli#555). Exec wrote the name as given and +// mxbuild refused it; check had resolved neither an input widget's Attribute +// nor a template parameter. where names the template parameter ("" for the +// widget's own Attribute). +func (v *attributeScopeValidator) reportMissingAttribute(w *ast.WidgetV3, where, path, entity, name string) { + if auditMembers[strings.ToLower(name)] { + return + } + var have []string + for _, e := range v.types.chain(entity) { + for _, a := range v.types.attrs[e] { + have = append(have, a.name) + } + } + sort.Strings(have) + const max = 12 + list := strings.Join(have, ", ") + if len(have) > max { + list = fmt.Sprintf("%s and %d more", strings.Join(have[:max], ", "), len(have)-max) + } + if list == "" { + list = "none" + } + subject := fmt.Sprintf("widget `%s` (%s)", w.Name, strings.ToLower(w.Type)) + if where != "" { + subject += " " + where + } + v.errs = append(v.errs, fmt.Sprintf( + "%s binds `%s`, but %s has no attribute %s (attributes: %s) — mxbuild reports CE1613 "+ + "\"The selected attribute '%s.%s' no longer exists\"", + subject, path, entity, name, list, entity, name)) +} + +// checkTemplateParams judges the attribute bindings among a widget's template +// parameters (ContentParams, CaptionParams) the way checkInputBinding judges an +// input's Attribute: against the entity the enclosing data container supplies, +// the context exec qualifies a bare name with (ako/mxcli#555). An expression, +// a $variable path, or a context this walk cannot resolve is not judged. +func (v *attributeScopeValidator) checkTemplateParams(w *ast.WidgetV3, enclosing dataContext) { + if enclosing.unresolved || len(enclosing.entities) == 0 { + return + } + for _, set := range []struct { + key string + params []ast.ParamAssignmentV3 + }{{"ContentParams", w.GetContentParams()}, {"CaptionParams", w.GetCaptionParams()}} { + for _, p := range set.params { + raw, ok := p.Value.(string) + if !ok || isTemplateExpression(raw) { + continue + } + path := strings.TrimPrefix(strings.TrimSpace(raw), "$currentObject/") + if path == "" || strings.HasPrefix(path, "$") { + continue + } + entity := enclosing.entities[len(enclosing.entities)-1] + segs := strings.Split(path, "/") + resolved := true + for _, seg := range segs[:len(segs)-1] { + _, next, ok := v.types.follow(seg, entity) + if !ok { + resolved = false + break + } + entity = next + } + name := segs[len(segs)-1] + if !resolved || name == "" || strings.Contains(name, ".") { + continue + } + if _, _, found, known := v.types.lookup(entity, name); !known || found { + continue + } + if _, _, isAssoc := v.types.follow(name, entity); isAssoc { + continue + } + v.reportMissingAttribute(w, fmt.Sprintf("%s {%d}", set.key, p.Index), raw, entity, name) + } + } +} + func anPrefix(kind string) string { if strings.ContainsAny(kind[:1], "AEIOU") { return "n" diff --git a/mdl/executor/validate_widget_attribute_type_test.go b/mdl/executor/validate_widget_attribute_type_test.go index f9f694ded..eba69ac0e 100644 --- a/mdl/executor/validate_widget_attribute_type_test.go +++ b/mdl/executor/validate_widget_attribute_type_test.go @@ -215,15 +215,17 @@ create page Sales.P2 (title: 'P', layout: Atlas_Core.Atlas_Default, params: ($Sh } // What the walk cannot place is not judged: an association data source, a page -// variable, an attribute nobody declares. +// variable. An attribute the entity does not have is not a TYPE error — there is +// no type to judge — but it is the CE1613 of ako/mxcli#555, reported once. func TestCheck_AttributeTypeUnjudgedWhenUnknown(t *testing.T) { ctx := typeCheckCtx(t, "Yes") errs := typeErrs(t, ctx, `dataview dv (datasource: $Thing) { dataview dv2 (datasource: association Thing_Other) { textbox tb1 (label: 'x', attribute: Shade) } textbox tb2 (label: 'x', attribute: NoSuchAttribute) }`, nil) - if len(errs) != 0 { - t.Fatalf("want nothing judged, got %q", errs) + if len(errs) != 1 || !strings.Contains(errs[0], "widget `tb2`") || + !strings.Contains(errs[0], "has no attribute NoSuchAttribute") || strings.Contains(errs[0], widgetAttributeTypeRule) { + t.Fatalf("want only tb2's missing-attribute CE1613, no type error, got %q", errs) } } From 693857cc02a94340486671e2d31aff9f46cac08d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 22:04:11 +0000 Subject: [PATCH 8/8] test: fixture copies leave out the .mxcli caches tests write in place The nightly's 11.12.2 leg failed in setup, not in what it tests: TestCreateMicroflow_NotesRefusedOnFullConstructor (mdl/backend/mcp) with "copy fixture: open .mxcli/cache/unit-contents.db-journal: no such file or directory". The same commit passed on every other leg. Since the unit-contents store (ako/mxcli#1804) a reader writes .mxcli/cache/unit-contents.db next to the project it opens, and tests that open testdata/expr-checker, testdata/pedapp or the v1-project fixture in place write it into the checked-in fixture, its SQLite journal coming and going with each write. go test runs packages in parallel, so another package's whole-directory copy listed the journal and then could not open it. Nothing under a fixture's .mxcli is tracked. testutil.CopyFixture is os.CopyFS over a view that hides every .mxcli, so a copy holds what the fixture commits, as a fresh checkout does. Every whole-fixture os.CopyFS in tests (25) uses it, and so do cmd/mxcli's copyTree and copyFixtureTree. Copies of mprcontents alone are unchanged. Control: an fs.FS whose listing names a journal that Open reports missing makes plain os.CopyFS fail with the nightly's error; CopyFixture copies it, and fails the same way with its filter stubbed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GeAhBjRBm5vN2ik3ddvVWW --- ...fixture-copy-races-unit-cache-journal.json | 1 + cmd/mxcli/brain_before_project_test.go | 3 +- cmd/mxcli/cmd_fmt_upgrade_test.go | 19 ++---- cmd/mxcli/cmd_layout_flows_test.go | 4 +- cmd/mxcli/docker/update_widgets_test.go | 5 +- cmd/mxcli/exec_atomic_test.go | 20 ++---- .../exec_preflight_timeline_1762_test.go | 3 +- cmd/mxcli/marketplace/snapshot_test.go | 4 +- cmd/mxcli/mcp_exec_no_file_write_test.go | 4 +- cmd/mxcli/mcp_trace_test.go | 3 +- cmd/mxcli/project_reader_test.go | 5 +- cmd/mxcli/show_version_format_test.go | 4 +- internal/testutil/fixture.go | 45 ++++++++++++++ internal/testutil/fixture_test.go | 61 +++++++++++++++++++ mdl/backend/mcp/connect_reads_test.go | 4 +- .../modelsdk/domainmodel_get_cost_test.go | 4 +- .../modelsdk/domainmodel_write_test.go | 4 +- .../modelsdk/flow_version_keys_test.go | 4 +- .../modelsdk/issue1373_property_sets_test.go | 4 +- .../issue1805_odata_property_sets_test.go | 3 +- .../issue627_update_attribute_guid_test.go | 4 +- .../issue628_move_cross_assoc_test.go | 4 +- .../settings_create_configuration_test.go | 4 +- .../modelsdk/widget_custom_find_test.go | 5 +- mdl/backend/modelsdk/workflow_esp_test.go | 3 +- mdl/executor/typecheck_test.go | 4 +- mdl/executor/validate_project_reader_test.go | 4 +- .../validate_widget_object_property_test.go | 3 +- mdl/roundtrip/viewentity_identity_test.go | 4 +- 29 files changed, 168 insertions(+), 71 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/other/2026-11-04-fixture-copy-races-unit-cache-journal.json create mode 100644 internal/testutil/fixture.go create mode 100644 internal/testutil/fixture_test.go diff --git a/.claude/skills/fix-issue/findings/other/2026-11-04-fixture-copy-races-unit-cache-journal.json b/.claude/skills/fix-issue/findings/other/2026-11-04-fixture-copy-races-unit-cache-journal.json new file mode 100644 index 000000000..0d8378492 --- /dev/null +++ b/.claude/skills/fix-issue/findings/other/2026-11-04-fixture-copy-races-unit-cache-journal.json @@ -0,0 +1 @@ +{"area":"other","date":"2026-11-04","symptom":"A unit test fails at random on one nightly leg, in setup, not in what it tests: `TestCreateMicroflow_NotesRefusedOnFullConstructor` (mdl/backend/mcp) with `copy fixture: open .mxcli/cache/unit-contents.db-journal: no such file or directory`. The same commit passes on every other leg and on re-run","cause":"A cross-package race on the checked-in fixtures, not a product defect. Since the unit-contents store (ako/mxcli#1804) any reader that opens a project writes `.mxcli/cache/unit-contents.db` next to it, and tests that open testdata/expr-checker, testdata/pedapp or modelsdk/mpr/testdata/v1-project in place write it into the fixture; its SQLite journal appears and disappears with each write. go test runs packages in parallel, so another package's whole-directory `os.CopyFS` (25 tests) or hand-written walk listed the journal and then failed to open it. Nothing under a fixture's .mxcli is tracked","file":"`internal/testutil/fixture.go` (`CopyFixture`: os.CopyFS over a view that hides every .mxcli), used by every whole-fixture copy in tests, and by cmd/mxcli's copyTree and copyFixtureTree","insight":"A cache that writes next to the project turns every checked-in fixture into shared mutable state between test packages; any whole-directory copy of one is then a race whose odds grow with the suite. Copy what the fixture commits, never its .mxcli, which also makes the copy what a fresh checkout holds whichever tests ran first (exec_atomic's hashTree already ignored .mxcli for the same reason). After a full `make test`, `find . -name .mxcli -path '*testdata*'` lists the fixtures being written in place. The race is reproduced deterministically by an fs.FS whose listing names a journal that Open reports missing: plain os.CopyFS fails with the nightly's error, CopyFixture copies; with its filter stubbed it fails the same way","refs":["ako/mxcli nightly run 41801123003 (test (Mendix 11.12.2), Unit tests)","ako/mxcli#1804"],"test":"internal/testutil/fixture_test.go"} diff --git a/cmd/mxcli/brain_before_project_test.go b/cmd/mxcli/brain_before_project_test.go index 1aeaed71a..2024d6d2a 100644 --- a/cmd/mxcli/brain_before_project_test.go +++ b/cmd/mxcli/brain_before_project_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/mendixlabs/mxcli/cmd/mxcli/brain" + "github.com/mendixlabs/mxcli/internal/testutil" ) // A brain can be born before its project (ako/mxcli#1949). These tests pin @@ -190,7 +191,7 @@ func TestBrainBornBeforeTheProjectIsCountedAfterIt(t *testing.T) { t.Fatalf("checkNewProjectDestination = %v, %v; want adopt", adopt, err) } stage := filepath.Join(t.TempDir(), "stage") - if err := os.CopyFS(stage, os.DirFS(filepath.Join("..", "..", "testdata", "pedapp"))); err != nil { + if err := testutil.CopyFixture(stage, filepath.Join("..", "..", "testdata", "pedapp")); err != nil { t.Fatal(err) } if err := moveProject(stage, dir); err != nil { diff --git a/cmd/mxcli/cmd_fmt_upgrade_test.go b/cmd/mxcli/cmd_fmt_upgrade_test.go index 618137047..0457ca314 100644 --- a/cmd/mxcli/cmd_fmt_upgrade_test.go +++ b/cmd/mxcli/cmd_fmt_upgrade_test.go @@ -12,6 +12,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/executor" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -226,22 +227,10 @@ func TestFmtUpgrade_FindOnACallResultReadsTheProject(t *testing.T) { } } +// copyTree copies a fixture project, leaving out the .mxcli caches other +// packages' tests write into it in place (testutil.CopyFixture). func copyTree(src, dst string) error { - return filepath.Walk(src, func(p string, info os.FileInfo, err error) error { - if err != nil { - return err - } - rel, _ := filepath.Rel(src, p) - target := filepath.Join(dst, rel) - if info.IsDir() { - return os.MkdirAll(target, 0o755) - } - data, err := os.ReadFile(p) - if err != nil { - return err - } - return os.WriteFile(target, data, 0o644) - }) + return testutil.CopyFixture(dst, src) } // ako/mxcli#873: without a project, a bare commit in a `create or modify` diff --git a/cmd/mxcli/cmd_layout_flows_test.go b/cmd/mxcli/cmd_layout_flows_test.go index 0b6c0a170..713f5e49a 100644 --- a/cmd/mxcli/cmd_layout_flows_test.go +++ b/cmd/mxcli/cmd_layout_flows_test.go @@ -4,11 +4,11 @@ package main import ( "bytes" - "os" "path/filepath" "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -18,7 +18,7 @@ import ( func flowLayoutFixture(t *testing.T) (*bytes.Buffer, func(args ...string) error) { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } exec, logger := newLoggedExecutor("test") diff --git a/cmd/mxcli/docker/update_widgets_test.go b/cmd/mxcli/docker/update_widgets_test.go index ef47c53fe..b15bb1579 100644 --- a/cmd/mxcli/docker/update_widgets_test.go +++ b/cmd/mxcli/docker/update_widgets_test.go @@ -18,6 +18,7 @@ import ( "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/types" ) @@ -37,7 +38,7 @@ func storageVersion(t *testing.T, mprPath string) types.MPRVersion { func v2Fixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/expr-checker"); err != nil { t.Fatalf("copy v2 fixture: %v", err) } p := filepath.Join(dst, "minimal.mpr") @@ -51,7 +52,7 @@ func v2Fixture(t *testing.T) string { func v1Fixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../modelsdk/mpr/testdata/v1-project")); err != nil { + if err := testutil.CopyFixture(dst, "../../../modelsdk/mpr/testdata/v1-project"); err != nil { t.Fatalf("copy v1 fixture: %v", err) } p := filepath.Join(dst, "App.mpr") diff --git a/cmd/mxcli/exec_atomic_test.go b/cmd/mxcli/exec_atomic_test.go index 86b5e89c0..cb7d9a3a2 100644 --- a/cmd/mxcli/exec_atomic_test.go +++ b/cmd/mxcli/exec_atomic_test.go @@ -13,6 +13,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/executor" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -110,22 +111,9 @@ func runScriptOn(t *testing.T, mprPath, script string) error { func copyFixtureTree(t *testing.T, src string) string { t.Helper() dst := t.TempDir() - err := filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - rel, _ := filepath.Rel(src, p) - target := filepath.Join(dst, rel) - if d.IsDir() { - return os.MkdirAll(target, 0o755) - } - data, err := os.ReadFile(p) - if err != nil { - return err - } - return os.WriteFile(target, data, 0o644) - }) - if err != nil { + // Not .mxcli: other packages' tests write their caches into the fixture in + // place (testutil.CopyFixture), and hashTree ignores it anyway. + if err := testutil.CopyFixture(dst, src); err != nil { t.Fatal(err) } return dst diff --git a/cmd/mxcli/exec_preflight_timeline_1762_test.go b/cmd/mxcli/exec_preflight_timeline_1762_test.go index e948758ed..e52f7f0ec 100644 --- a/cmd/mxcli/exec_preflight_timeline_1762_test.go +++ b/cmd/mxcli/exec_preflight_timeline_1762_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" @@ -62,7 +63,7 @@ func TestExecPreflightRefusesHiddenTimelineEnumerations(t *testing.T) { t.Skipf("expr-checker fixture not found: %v", err) } dir := t.TempDir() - if err := os.CopyFS(dir, os.DirFS(src)); err != nil { + if err := testutil.CopyFixture(dir, src); err != nil { t.Fatal(err) } mpr := filepath.Join(dir, "minimal.mpr") diff --git a/cmd/mxcli/marketplace/snapshot_test.go b/cmd/mxcli/marketplace/snapshot_test.go index 4b2639dc1..1098e84ac 100644 --- a/cmd/mxcli/marketplace/snapshot_test.go +++ b/cmd/mxcli/marketplace/snapshot_test.go @@ -4,10 +4,10 @@ package marketplace import ( "bytes" - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" @@ -26,7 +26,7 @@ const fixtureDir = "../../../testdata/expr-checker" func copyFixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS(fixtureDir)); err != nil { + if err := testutil.CopyFixture(dst, fixtureDir); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/cmd/mxcli/mcp_exec_no_file_write_test.go b/cmd/mxcli/mcp_exec_no_file_write_test.go index 06985605a..a85bb45f7 100644 --- a/cmd/mxcli/mcp_exec_no_file_write_test.go +++ b/cmd/mxcli/mcp_exec_no_file_write_test.go @@ -11,6 +11,8 @@ import ( "sort" "strings" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // mendixlabs/mxcli#1684 (ako/mxcli#1833): "`mxcli exec --mcp ` @@ -35,7 +37,7 @@ const mcpProbeConstant = "create constant MyFirstModule.LiveProbe (Type: String, func mcpLockedProject(t *testing.T, lock bool) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } if lock { diff --git a/cmd/mxcli/mcp_trace_test.go b/cmd/mxcli/mcp_trace_test.go index 9de5c2a0a..7613e4775 100644 --- a/cmd/mxcli/mcp_trace_test.go +++ b/cmd/mxcli/mcp_trace_test.go @@ -14,6 +14,7 @@ import ( "sync" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -112,7 +113,7 @@ func TestMCPTrace_ListsThePEDCallsOfAWrite(t *testing.T) { srv, calls := fakeStudioPro(t) withMCPFlags(t, srv.URL+"/mcp", strings.TrimPrefix(srv.URL, "http://"), true) dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } diff --git a/cmd/mxcli/project_reader_test.go b/cmd/mxcli/project_reader_test.go index 3d2fc937b..eb9116ccc 100644 --- a/cmd/mxcli/project_reader_test.go +++ b/cmd/mxcli/project_reader_test.go @@ -13,15 +13,16 @@ package main // silently depend on. import ( - "os" "path/filepath" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) func fixtureProject(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/cmd/mxcli/show_version_format_test.go b/cmd/mxcli/show_version_format_test.go index 2b77f6df6..2a9e770cf 100644 --- a/cmd/mxcli/show_version_format_test.go +++ b/cmd/mxcli/show_version_format_test.go @@ -7,6 +7,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // SHOW VERSION printed `MPR Format: v2` for an MPR v1 project — a single .mpr @@ -29,7 +31,7 @@ func TestShowVersionReportsTheStorageFormatOnDisk(t *testing.T) { } v2dir := t.TempDir() - if err := os.CopyFS(v2dir, os.DirFS(filepath.Join("..", "..", "testdata", "testapp-views"))); err != nil { + if err := testutil.CopyFixture(v2dir, filepath.Join("..", "..", "testdata", "testapp-views")); err != nil { t.Fatalf("copy v2 fixture: %v", err) } v2 := filepath.Join(v2dir, "TestApp.mpr") diff --git a/internal/testutil/fixture.go b/internal/testutil/fixture.go new file mode 100644 index 000000000..fa5288083 --- /dev/null +++ b/internal/testutil/fixture.go @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testutil + +import ( + "io/fs" + "os" +) + +// CopyFixture copies the fixture project at src into dst the way os.CopyFS +// does, leaving out every .mxcli directory. +// +// .mxcli holds mxcli's own caches, never anything a fixture commits: nothing +// under it is tracked. But a test that opens a fixture in place writes them +// there (the unit-contents store, ako/mxcli#1804, a SQLite file whose journal +// comes and goes with each write), and go test runs packages in parallel. A +// whole-directory copy in another package then lists a journal that is gone by +// the time it opens it, and fails with +// +// open .mxcli/cache/unit-contents.db-journal: no such file or directory +// +// (seen on the nightly in mdl/backend/mcp). Leaving .mxcli out also makes the +// copy what a fresh checkout holds, whichever tests ran first. +func CopyFixture(dst, src string) error { + return copyFixtureFS(dst, os.DirFS(src)) +} + +func copyFixtureFS(dst string, fsys fs.FS) error { + return os.CopyFS(dst, withoutMxcli{fsys}) +} + +// withoutMxcli hides every .mxcli entry from the directory listings os.CopyFS +// walks. +type withoutMxcli struct{ fs.FS } + +func (f withoutMxcli) ReadDir(name string) ([]fs.DirEntry, error) { + entries, err := fs.ReadDir(f.FS, name) + kept := entries[:0] + for _, e := range entries { + if e.Name() != ".mxcli" { + kept = append(kept, e) + } + } + return kept, err +} diff --git a/internal/testutil/fixture_test.go b/internal/testutil/fixture_test.go new file mode 100644 index 000000000..67ae84384 --- /dev/null +++ b/internal/testutil/fixture_test.go @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testutil + +import ( + "errors" + "io/fs" + "os" + "path/filepath" + "testing" + "testing/fstest" +) + +// vanishingJournal is a fixture caught mid-write by another package's test: its +// listing still names the cache's journal, which is gone when the copy opens it. +type vanishingJournal struct{ fstest.MapFS } + +const journal = ".mxcli/cache/unit-contents.db-journal" + +func (v vanishingJournal) Open(name string) (fs.File, error) { + if name == journal { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + return v.MapFS.Open(name) +} + +func fixtureWithJournal() vanishingJournal { + return vanishingJournal{fstest.MapFS{ + "minimal.mpr": {Data: []byte("mpr")}, + "mprcontents/ab/cd/unit.mxunit": {Data: []byte("unit")}, + ".mxcli/cache/unit-contents.db": {Data: []byte("cache")}, + journal: {Data: []byte("journal")}, + "widgets/.mxcli/widgets/x.def.json": {Data: []byte("def")}, + }} +} + +// The nightly's symptom, reproduced deterministically: a plain os.CopyFS of the +// fixture fails on the journal another test's write has already removed. +func TestCopyFixture_Control_PlainCopyFailsOnAVanishedJournal(t *testing.T) { + err := os.CopyFS(t.TempDir(), fixtureWithJournal()) + if !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("os.CopyFS = %v, want the vanished journal's not-exist error", err) + } +} + +func TestCopyFixture_LeavesOutEveryMxcliDirectory(t *testing.T) { + dst := t.TempDir() + if err := copyFixtureFS(dst, fixtureWithJournal()); err != nil { + t.Fatalf("copy: %v", err) + } + for _, rel := range []string{"minimal.mpr", "mprcontents/ab/cd/unit.mxunit"} { + if _, err := os.Stat(filepath.Join(dst, filepath.FromSlash(rel))); err != nil { + t.Errorf("the project's %s was not copied: %v", rel, err) + } + } + for _, rel := range []string{".mxcli", "widgets/.mxcli"} { + if _, err := os.Stat(filepath.Join(dst, filepath.FromSlash(rel))); !os.IsNotExist(err) { + t.Errorf("%s was copied (stat err %v); it holds caches, never the fixture", rel, err) + } + } +} diff --git a/mdl/backend/mcp/connect_reads_test.go b/mdl/backend/mcp/connect_reads_test.go index 35add397e..e3d6bb184 100644 --- a/mdl/backend/mcp/connect_reads_test.go +++ b/mdl/backend/mcp/connect_reads_test.go @@ -12,11 +12,11 @@ package mcp // half goes over MCP to Studio Pro and is covered by the fakePED tests around it. import ( - "os" "path/filepath" "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/domainmodel" @@ -28,7 +28,7 @@ import ( func localProject(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/mdl/backend/modelsdk/domainmodel_get_cost_test.go b/mdl/backend/modelsdk/domainmodel_get_cost_test.go index 49db6ac55..526c945f4 100644 --- a/mdl/backend/modelsdk/domainmodel_get_cost_test.go +++ b/mdl/backend/modelsdk/domainmodel_get_cost_test.go @@ -3,11 +3,11 @@ package modelsdkbackend import ( - "os" "path/filepath" "reflect" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/domainmodel" ) @@ -104,7 +104,7 @@ func TestGetDomainModel_AgreesWithTheFullListing(t *testing.T) { func copyV1Project(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../modelsdk/mpr/testdata/v1-project")); err != nil { + if err := testutil.CopyFixture(dst, "../../../modelsdk/mpr/testdata/v1-project"); err != nil { t.Fatalf("copy v1 fixture: %v", err) } return filepath.Join(dst, "App.mpr") diff --git a/mdl/backend/modelsdk/domainmodel_write_test.go b/mdl/backend/modelsdk/domainmodel_write_test.go index b66f13781..964e42482 100644 --- a/mdl/backend/modelsdk/domainmodel_write_test.go +++ b/mdl/backend/modelsdk/domainmodel_write_test.go @@ -3,10 +3,10 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/sdk/domainmodel" ) @@ -15,7 +15,7 @@ import ( func copyFixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/mdl/backend/modelsdk/flow_version_keys_test.go b/mdl/backend/modelsdk/flow_version_keys_test.go index 1203b2d2d..07b823c88 100644 --- a/mdl/backend/modelsdk/flow_version_keys_test.go +++ b/mdl/backend/modelsdk/flow_version_keys_test.go @@ -3,10 +3,10 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/microflows" @@ -40,7 +40,7 @@ import ( func copyV9Fixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../modelsdk/mpr/testdata/v1-project")); err != nil { + if err := testutil.CopyFixture(dst, "../../../modelsdk/mpr/testdata/v1-project"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "App.mpr") diff --git a/mdl/backend/modelsdk/issue1373_property_sets_test.go b/mdl/backend/modelsdk/issue1373_property_sets_test.go index 875ccee8f..53df2a813 100644 --- a/mdl/backend/modelsdk/issue1373_property_sets_test.go +++ b/mdl/backend/modelsdk/issue1373_property_sets_test.go @@ -3,12 +3,12 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" bsonv2 "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/modelsdk/codec" "github.com/mendixlabs/mxcli/sdk/microflows" @@ -27,7 +27,7 @@ import ( // Documentation — the key whose absence made `mx diff` throw. func TestIssue1373_RewrittenLoopKeepsItsPropertySet(t *testing.T) { dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/testapp-views")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/testapp-views"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "TestApp.mpr") diff --git a/mdl/backend/modelsdk/issue1805_odata_property_sets_test.go b/mdl/backend/modelsdk/issue1805_odata_property_sets_test.go index d39a63e12..d77a3a87e 100644 --- a/mdl/backend/modelsdk/issue1805_odata_property_sets_test.go +++ b/mdl/backend/modelsdk/issue1805_odata_property_sets_test.go @@ -11,6 +11,7 @@ import ( "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/modelsdk/codec" mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr" @@ -166,7 +167,7 @@ func TestIssue1805_PublishedMicroflowWritesItsParametersWhenThereAreNone(t *test // name the elements of a type disagree on. func TestIssue1805_PublishedServiceCensusHasNoMixedNames(t *testing.T) { dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/testapp-views")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/testapp-views"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "TestApp.mpr") diff --git a/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go b/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go index f73ef0c0b..c6ef6a97b 100644 --- a/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go +++ b/mdl/backend/modelsdk/issue627_update_attribute_guid_test.go @@ -3,10 +3,10 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/domainmodel" ) @@ -17,7 +17,7 @@ import ( func copyPedApp(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/pedapp")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/pedapp"); err != nil { t.Fatalf("copy PedApp fixture: %v", err) } return filepath.Join(dst, "PedApp.mpr") diff --git a/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go b/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go index 180a4dc9a..2b1cd8586 100644 --- a/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go +++ b/mdl/backend/modelsdk/issue628_move_cross_assoc_test.go @@ -3,11 +3,11 @@ package modelsdkbackend import ( - "os" "path/filepath" "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" genDm "github.com/mendixlabs/mxcli/modelsdk/gen/domainmodels" ) @@ -298,7 +298,7 @@ func assertNoDanglingAssociations(t *testing.T, b *Backend) { func copyPedAppFixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/pedapp")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/pedapp"); err != nil { t.Fatalf("copy PedApp fixture: %v", err) } return filepath.Join(dst, "PedApp.mpr") diff --git a/mdl/backend/modelsdk/settings_create_configuration_test.go b/mdl/backend/modelsdk/settings_create_configuration_test.go index 72ad85808..e9a005e63 100644 --- a/mdl/backend/modelsdk/settings_create_configuration_test.go +++ b/mdl/backend/modelsdk/settings_create_configuration_test.go @@ -3,10 +3,10 @@ package modelsdkbackend import ( - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" ) @@ -17,7 +17,7 @@ import ( // 11.6 and 11.13 fixtures have no such part, which is why it went unnoticed. func TestUpdateProjectSettings_CreateConfigurationOn1114(t *testing.T) { dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/testapp-views")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/testapp-views"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "TestApp.mpr") diff --git a/mdl/backend/modelsdk/widget_custom_find_test.go b/mdl/backend/modelsdk/widget_custom_find_test.go index 16732ce85..3b5251e35 100644 --- a/mdl/backend/modelsdk/widget_custom_find_test.go +++ b/mdl/backend/modelsdk/widget_custom_find_test.go @@ -8,11 +8,12 @@ package modelsdkbackend // — the method would quietly go back to erroring. import ( - "os" "path/filepath" "testing" "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // imageWidgetID is in the shared fixture; combobox and datagrid are too. @@ -21,7 +22,7 @@ const imageWidgetID = "com.mendix.widget.web.image.Image" func customWidgetFixture(t *testing.T) *Backend { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } b := New() diff --git a/mdl/backend/modelsdk/workflow_esp_test.go b/mdl/backend/modelsdk/workflow_esp_test.go index 20c8e1278..bf9560b24 100644 --- a/mdl/backend/modelsdk/workflow_esp_test.go +++ b/mdl/backend/modelsdk/workflow_esp_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" genWf "github.com/mendixlabs/mxcli/modelsdk/gen/workflows" "github.com/mendixlabs/mxcli/sdk/workflows" "go.mongodb.org/mongo-driver/v2/bson" @@ -105,7 +106,7 @@ func TestCreateWorkflow_EventSubProcessesRoundTrip(t *testing.T) { // fixture: on the 11.6 one the storage layer refuses the workflow // (TestCreateWorkflow_EventSubProcessesRefusedBeforeTheirVersion). dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../../testdata/testapp-views")); err != nil { + if err := testutil.CopyFixture(dst, "../../../testdata/testapp-views"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "TestApp.mpr") diff --git a/mdl/executor/typecheck_test.go b/mdl/executor/typecheck_test.go index 7314a5922..2e5b53acc 100644 --- a/mdl/executor/typecheck_test.go +++ b/mdl/executor/typecheck_test.go @@ -4,11 +4,11 @@ package executor import ( "bytes" - "os" "path/filepath" "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" "github.com/mendixlabs/mxcli/mdl/linter" @@ -27,7 +27,7 @@ import ( func typeCheckFixture(t *testing.T) *Executor { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } proj := filepath.Join(dst, "minimal.mpr") diff --git a/mdl/executor/validate_project_reader_test.go b/mdl/executor/validate_project_reader_test.go index 08490ea56..a30fb6965 100644 --- a/mdl/executor/validate_project_reader_test.go +++ b/mdl/executor/validate_project_reader_test.go @@ -14,10 +14,10 @@ package executor import ( "bytes" - "os" "path/filepath" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" "github.com/mendixlabs/mxcli/mdl/visitor" @@ -27,7 +27,7 @@ import ( func projectFixture(t *testing.T) string { t.Helper() dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS("../../testdata/expr-checker")); err != nil { + if err := testutil.CopyFixture(dst, "../../testdata/expr-checker"); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, "minimal.mpr") diff --git a/mdl/executor/validate_widget_object_property_test.go b/mdl/executor/validate_widget_object_property_test.go index 56348e3b6..11d256b60 100644 --- a/mdl/executor/validate_widget_object_property_test.go +++ b/mdl/executor/validate_widget_object_property_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/visitor" @@ -26,7 +27,7 @@ func fixtureProjectWithDefs(t *testing.T) string { t.Helper() src := filepath.Dir(fixtureProject(t)) dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS(src)); err != nil { + if err := testutil.CopyFixture(dst, src); err != nil { t.Fatalf("copy fixture: %v", err) } // Start from no definitions whatever the developer's tree holds, so the diff --git a/mdl/roundtrip/viewentity_identity_test.go b/mdl/roundtrip/viewentity_identity_test.go index fc6fa615f..5cc6c1980 100644 --- a/mdl/roundtrip/viewentity_identity_test.go +++ b/mdl/roundtrip/viewentity_identity_test.go @@ -7,12 +7,12 @@ package roundtrip import ( "bytes" "encoding/hex" - "os" "path/filepath" "testing" "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend" modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" @@ -153,7 +153,7 @@ func copyViewsFixture(t *testing.T) string { t.Fatal(err) } dst := t.TempDir() - if err := os.CopyFS(dst, os.DirFS(src)); err != nil { + if err := testutil.CopyFixture(dst, src); err != nil { t.Fatalf("copy fixture: %v", err) } return filepath.Join(dst, viewsFixtureMPR)