From 8b11d0429889eb11e7800d84724f58cd5cb20d9e Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 19:54:13 +0000 Subject: [PATCH 01/13] fix(describe): DESCRIBE FRAGMENT FROM PAGE/SNIPPET found no widget, ever MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The visitor stores DescribeFragmentFromStmt.ContainerType as "PAGE"/"SNIPPET" while describeFragmentFrom switched on "page"/"snippet" with no default, so neither branch ran and every widget was reported missing ("not found in page M.P" — without even naming the widget). Same casing split as ALTER PAGE (#402) and ALTER STYLING (#631). Normalise with strings.ToLower (the convention of the other consumers), make an unrecognised container type an error instead of an empty widget list, and name the widget in the not-found message. The new tests parse the statement and dispatch it through the registry, so they pin the visitor/executor casing contract that a hand-built lowercase AST could not see. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../describe-fragment-from-container-case.mdl | 32 +++++ mdl/executor/cmd_fragments.go | 13 +- mdl/executor/cmd_fragments_from_test.go | 112 ++++++++++++++++++ 4 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 mdl-examples/bug-tests/describe-fragment-from-container-case.mdl create mode 100644 mdl/executor/cmd_fragments_from_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..df18c6f9b 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area": "mdl/executor", "date": "2026-09-26", "symptom": "`describe fragment from page M.P widget w` (and `from snippet`) fails for every container and every widget — including ones `describe page` prints — with `not found in page M.P`, a message that does not even name the widget", "cause": "Visitor stores DescribeFragmentFromStmt.ContainerType as \"PAGE\"/\"SNIPPET\"; describeFragmentFrom switched on \"page\"/\"snippet\" with no default, so neither branch ran, the widget list stayed empty, and the fall-through reported the widget missing. Third instance of this split: ALTER PAGE (#402), DESCRIBE/ALTER STYLING (#631)", "file": "`mdl/executor/cmd_fragments.go` (`describeFragmentFrom`)", "insight": "The mismatch hid behind a plausible error because a switch on the discriminator had no default: an unmatched container type looked like an empty container, and an empty container looks like a missing widget. Normalise with strings.ToLower where the discriminator is consumed (the house convention — cmd_styling, cmd_alter_page, validate_alter_* all do) AND make the default an error, so the next casing drift fails loudly instead of reporting the wrong thing. The existing mock tests hand-built the AST and so agreed with the handler; only a test that goes visitor.Build → NewRegistry().Dispatch pins the contract between the two layers (cmd_fragments_from_test.go). Verified on Evora: Administration.Account_Edit/textBox6 and AgentCommons.Snippet_Agent_Details/dataView7 now describe", "refs": ["#402", "#631"]} diff --git a/mdl-examples/bug-tests/describe-fragment-from-container-case.mdl b/mdl-examples/bug-tests/describe-fragment-from-container-case.mdl new file mode 100644 index 000000000..eaa9862fe --- /dev/null +++ b/mdl-examples/bug-tests/describe-fragment-from-container-case.mdl @@ -0,0 +1,32 @@ +-- DESCRIBE FRAGMENT FROM PAGE/SNIPPET always failed — found by an +-- agent-orientation audit on a large app (Evora Factory Management). +-- +-- REPORTED SYMPTOM +-- +-- describe fragment from page Administration.Account_Edit widget textBox6 +-- → Error: not found in page Administration.Account_Edit +-- +-- for every page, every snippet and every widget — including widgets that +-- `describe page` prints. The message also omitted the widget's name. +-- +-- CAUSE +-- +-- The visitor stores ContainerType as "PAGE"/"SNIPPET"; the handler in +-- mdl/executor/cmd_fragments.go switched on "page"/"snippet". Nothing +-- matched, the widget list stayed empty, and the fall-through reported the +-- widget as missing. Same casing split as ALTER STYLING (#631) and ALTER +-- PAGE (#402) before it. +-- +-- EXPECTED (run against a project that has these documents; the page is the +-- stock Administration module, so most apps carry it) +-- +-- 1. prints: textbox textBox6 (Label: 'Full name', Attribute: FullName) +-- 2. Error: widget noSuchWidget not found in page Administration.Account_Edit +-- 3. Error: page not found: Administration.NoSuchPage +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/describe-fragment-from-container-case.mdl -p app.mpr + +describe fragment from page Administration.Account_Edit widget textBox6; +describe fragment from page Administration.Account_Edit widget noSuchWidget; +describe fragment from page Administration.NoSuchPage widget textBox6; diff --git a/mdl/executor/cmd_fragments.go b/mdl/executor/cmd_fragments.go index 15b5782f0..7f50781fb 100644 --- a/mdl/executor/cmd_fragments.go +++ b/mdl/executor/cmd_fragments.go @@ -81,7 +81,13 @@ func describeFragmentFrom(ctx *ExecContext, s *ast.DescribeFragmentFromStmt) err var rawWidgets []rawWidget - switch s.ContainerType { + // The visitor stores ContainerType uppercase ("PAGE"/"SNIPPET"), as it + // does for DESCRIBE/ALTER STYLING and ALTER PAGE. Normalise here, and make + // an unrecognised value an error: falling through the switch leaves no + // widgets, which then reads as "widget not found" — the disguise this + // casing mismatch wore here and in cmd_styling.go before it. + containerType := strings.ToLower(s.ContainerType) + switch containerType { case "page": allPages, err := ctx.Backend.ListPages() if err != nil { @@ -119,12 +125,15 @@ func describeFragmentFrom(ctx *ExecContext, s *ast.DescribeFragmentFromStmt) err return mdlerrors.NewNotFound("snippet", s.ContainerName.String()) } rawWidgets = getSnippetWidgetsFromRaw(ctx, foundSnippet.ID) + + default: + return mdlerrors.NewUnsupported("describe fragment from: unsupported container type " + s.ContainerType) } // Find the widget by name target := findRawWidgetByName(rawWidgets, s.WidgetName) if target == nil { - return mdlerrors.NewNotFoundMsg("widget", s.WidgetName, fmt.Sprintf("not found in %s %s", strings.ToLower(s.ContainerType), s.ContainerName.String())) + return mdlerrors.NewNotFoundMsg("widget", s.WidgetName, fmt.Sprintf("widget %s not found in %s %s", s.WidgetName, containerType, s.ContainerName.String())) } // Output as MDL diff --git a/mdl/executor/cmd_fragments_from_test.go b/mdl/executor/cmd_fragments_from_test.go new file mode 100644 index 000000000..225a55874 --- /dev/null +++ b/mdl/executor/cmd_fragments_from_test.go @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// These tests parse the statement and dispatch it through the registry, +// rather than hand-building a DescribeFragmentFromStmt. The defect they pin +// was a casing contract between two layers: the visitor stores ContainerType +// as "PAGE"/"SNIPPET" and the handler switched on "page"/"snippet", so every +// `describe fragment from ...` fell through the switch with no widgets and +// reported the widget as missing. A test that builds the AST in lowercase +// agrees with the handler and cannot see that. + +func fragmentFromCtx(t *testing.T) (*ExecContext, *bytes.Buffer) { + t.Helper() + mod := mkModule("Shop") + pg := mkPage(mod.ID, "Product_Edit") + sn := mkSnippet(mod.ID, "Product_Card") + + widget := func(name string) map[string]any { + return map[string]any{ + "$Type": "Forms$DivContainer", + "Name": name, + "Widgets": []any{int32(2)}, + } + } + raw := map[model.ID]map[string]any{ + pg.ID: { + "$Type": "Forms$Page", + "FormCall": map[string]any{ + "Arguments": []any{int32(2), map[string]any{ + "Widgets": []any{int32(2), widget("pageBox")}, + }}, + }, + }, + sn.ID: { + "$Type": "Forms$Snippet", + "Widgets": []any{int32(2), widget("snippetBox")}, + }, + } + + ctx, buf := newMockCtx(t, withHierarchy(mkHierarchy(mod))) + mb := ctx.Backend.(*mock.MockBackend) + mb.ListPagesFunc = func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil } + mb.ListSnippetsFunc = func() ([]*pages.Snippet, error) { return []*pages.Snippet{sn}, nil } + mb.GetRawUnitFunc = func(id model.ID) (map[string]any, error) { return raw[id], nil } + return ctx, buf +} + +func runParsed(t *testing.T, ctx *ExecContext, src string) error { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + if len(prog.Statements) != 1 { + t.Fatalf("want 1 statement, got %d", len(prog.Statements)) + } + return NewRegistry().Dispatch(ctx, prog.Statements[0]) +} + +func TestDescribeFragmentFrom_Parsed(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"page", `describe fragment from page Shop.Product_Edit widget pageBox;`, "pageBox"}, + {"snippet", `describe fragment from snippet Shop.Product_Card widget snippetBox;`, "snippetBox"}, + {"uppercase keywords", `DESCRIBE FRAGMENT FROM PAGE Shop.Product_Edit WIDGET pageBox;`, "pageBox"}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx, buf := fragmentFromCtx(t) + if err := runParsed(t, ctx, tc.src); err != nil { + t.Fatalf("execute: %v", err) + } + if !strings.Contains(buf.String(), tc.want) { + t.Fatalf("output does not describe widget %q:\n%s", tc.want, buf.String()) + } + }) + } +} + +// A missing widget must name what was looked for — the widget and the +// container — so the reader can tell a typo from a wrong page. +func TestDescribeFragmentFrom_MissingWidgetNamesItAndContainer(t *testing.T) { + ctx, _ := fragmentFromCtx(t) + err := runParsed(t, ctx, `describe fragment from page Shop.Product_Edit widget noSuchBox;`) + if err == nil { + t.Fatal("want an error for a missing widget") + } + for _, want := range []string{"noSuchBox", "page", "Shop.Product_Edit"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err, want) + } + } +} + +// A missing container is its own error, not a missing widget. +func TestDescribeFragmentFrom_MissingContainer(t *testing.T) { + ctx, _ := fragmentFromCtx(t) + err := runParsed(t, ctx, `describe fragment from snippet Shop.NoSuchSnippet widget snippetBox;`) + if err == nil || !strings.Contains(err.Error(), "snippet not found") { + t.Fatalf("want snippet-not-found, got %v", err) + } +} From 94367426107957fa577f13dfd58d09531279a87e Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:00:15 +0000 Subject: [PATCH 02/13] fix(search): warn when the source index was never built MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `search` needs only a full catalog, which indexes string literals but not MDL source. On a project where `refresh catalog full source` had never run, the source half of every search came back empty with no word — read by agents as "no microflow/page mentions this". search now checks the build mode the catalog records (not the row count, so a built-but-empty index stays silent) and, below "source", prints a warning naming `refresh catalog full source` on a new ExecContext.Diagnostics writer (nil = stderr), keeping --format json stdout pure. The old unconditional "Tip: refresh catalog source" on stdout is replaced by it. Table format now delegates to execSearch up front instead of querying twice. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + cmd/mxcli/cmd_query.go | 3 + .../bug-tests/search-missing-source-index.mdl | 31 ++++ mdl/executor/cmd_catalog.go | 42 +++++- mdl/executor/cmd_search_source_index_test.go | 136 ++++++++++++++++++ mdl/executor/exec_context.go | 15 ++ 6 files changed, 224 insertions(+), 4 deletions(-) create mode 100644 mdl-examples/bug-tests/search-missing-source-index.mdl create mode 100644 mdl/executor/cmd_search_source_index_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..449b237f4 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area": "mdl/executor", "date": "2026-09-26", "symptom": "`search 'keyword'` (MDL and `mxcli search`, every format) on a project where `refresh catalog full source` was never run returns string-literal matches only, with no word that MDL source was not searched. An agent orienting on a large app (Evora Factory Management, 786 entities) read the empty source section as \"no microflow/page mentions this\". When nothing matched at all a tip did print — to stdout, and regardless of whether the index existed", "cause": "search calls ensureCatalog(full=true): full mode builds the strings FTS table but not source, and a full build is deliberately not promoted to source (source costs a describe of every document). The source query ran against an empty FTS table and `err == nil && Count > 0` treated \"never built\" and \"nothing matched\" identically", "file": "mdl/executor/cmd_catalog.go", "insight": "An empty result from a table that a cheaper build mode leaves unpopulated is the #1060 GRAPH_CYCLES shape again: the catalog already records its build_mode in catalog_meta (GetCacheInfo().BuildMode), so decide \"was this index built?\" from that marker, never from row count — a source-mode catalog with zero rows is a real answer and must stay silent. warnIfCatalogModeInsufficient did this for `select from CATALOG.SOURCE` but search built its own query and bypassed it; any command that queries a mode-gated table directly needs the same check. Route the warning to a diagnostics writer (ExecContext.Diagnostics, nil = stderr), not ctx.Output, or it corrupts --format json. The test fixture that makes this cheap: write a real catalog cache (SetCacheInfo with the stand-in .mpr's mtime + SaveToFile to /.mxcli/catalog.db) and ensureCatalog loads it with a MockBackend, no project needed", "refs": ["mendixlabs/mxcli#1060"], "rules": []} diff --git a/cmd/mxcli/cmd_query.go b/cmd/mxcli/cmd_query.go index d99b7fbf0..582438bca 100644 --- a/cmd/mxcli/cmd_query.go +++ b/cmd/mxcli/cmd_query.go @@ -212,6 +212,9 @@ var searchCmd = &cobra.Command{ Searches across string literals (captions, labels, messages) and MDL source definitions. Requires at least a FULL catalog build (done automatically). +MDL source is searched only once the source index exists — build it with + mxcli -p app.mpr -c "refresh catalog full source" +Until then a warning on stderr says only string literals were searched. Output Formats: table - Human-readable table (default) diff --git a/mdl-examples/bug-tests/search-missing-source-index.mdl b/mdl-examples/bug-tests/search-missing-source-index.mdl new file mode 100644 index 000000000..a62ffa875 --- /dev/null +++ b/mdl-examples/bug-tests/search-missing-source-index.mdl @@ -0,0 +1,31 @@ +-- ============================================================================ +-- search: say when the source index is missing +-- ============================================================================ +-- +-- Symptom (before fix): +-- `search 'keyword'` (MDL and `mxcli search`) needs only a FULL catalog, +-- which indexes string literals but not MDL source. On any project where +-- `refresh catalog full source` had never been run, CATALOG.SOURCE was +-- empty and the source half of every search came back empty with no word — +-- an agent read "no source matches" as "no microflow mentions this". When +-- strings DID match, there was no hint at all; when nothing matched, the tip +-- printed regardless of whether the index existed. +-- +-- After fix: +-- A catalog whose recorded build mode is below "source" prints, on stderr: +-- Warning: the source index (CATALOG.SOURCE) is not built (catalog mode: full) ... +-- Build it with: refresh catalog full source (CLI: mxcli -p ... -c "...") +-- A source-mode catalog never warns, even with zero matches (keyed off the +-- build mode, not the row count). `--format json` stdout stays pure JSON. +-- +-- Run against any project (needs a connection; the catalog is built on demand): +-- mxcli -p app.mpr -c "refresh catalog full" +-- mxcli -p app.mpr -c "search 'retrieve'" -- expect the warning on stderr +-- mxcli -p app.mpr -c "refresh catalog full source" +-- mxcli -p app.mpr -c "search 'retrieve'" -- expect Source Matches, no warning +-- mxcli search -p app.mpr "zzqq_no_match" --format json -- stdout: [] stderr: nothing +-- ============================================================================ + +refresh catalog full; + +search 'retrieve'; diff --git a/mdl/executor/cmd_catalog.go b/mdl/executor/cmd_catalog.go index 6d9be1256..961a19e41 100644 --- a/mdl/executor/cmd_catalog.go +++ b/mdl/executor/cmd_catalog.go @@ -1052,12 +1052,43 @@ func execSearch(ctx *ExecContext, stmt *ast.SearchStmt) error { if !found { fmt.Fprintln(ctx.Output, "No matches found.") - fmt.Fprintln(ctx.Output, "Tip: Use refresh catalog source to enable source-level search.") } + warnIfSourceIndexMissing(ctx) return nil } +// warnIfSourceIndexMissing says, on the diagnostics channel, that a search did +// not look at MDL source because CATALOG.SOURCE was never built. `search` needs +// only a full catalog, which has the strings index but not the source one, so +// without this the source half of every answer came back empty in silence — +// indistinguishable from "no microflow, page or expression mentions this". +// +// Keyed off the build mode the catalog records, never the row count: a +// source-mode catalog with no matching (or no) rows is a real answer, and +// counting rows would make "built, nothing matched" warn too. +func warnIfSourceIndexMissing(ctx *ExecContext) { + mode := "" + if ctx.Catalog != nil { + if info, err := ctx.Catalog.GetCacheInfo(); err == nil { + mode = info.BuildMode + } + } + if catalogModeRank(mode) >= catalogModeRank("source") { + return + } + if mode == "" { + mode = "unknown" + } + w := ctx.diagnostics() + fmt.Fprintf(w, "Warning: the source index (CATALOG.SOURCE) is not built (catalog mode: %s), so only string literals were searched — MDL source (microflow/nanoflow bodies, expressions, page and entity definitions) was not.\n", mode) + if ctx.MprPath != "" { + fmt.Fprintf(w, "Build it with: refresh catalog full source (CLI: mxcli -p %q -c \"refresh catalog full source\")\n", ctx.MprPath) + } else { + fmt.Fprintln(w, "Build it with: refresh catalog full source") + } +} + // escapeFTSQuery escapes special characters in FTS5 queries. // FTS5 treats characters like '/', '.', '-' as token separators. To make // queries like 'rest/companies' or 'Module.Entity' usable, we replace these @@ -1080,6 +1111,10 @@ func search(ctx *ExecContext, query, format string) error { return mdlerrors.NewNotConnected() } + if format != "names" && format != "json" { // "table" (default) + return execSearch(ctx, &ast.SearchStmt{Query: query}) + } + // Ensure catalog is built (at least full mode for strings table) if err := ensureCatalog(ctx, true); err != nil { return err @@ -1137,10 +1172,10 @@ func search(ctx *ExecContext, query, format string) error { if len(allResults) == 0 { if format != "json" { fmt.Fprintln(ctx.Output, "No matches found.") - fmt.Fprintln(ctx.Output, "Tip: Use refresh catalog source to enable source-level search.") } else { fmt.Fprintln(ctx.Output, "[]") } + warnIfSourceIndexMissing(ctx) return nil } @@ -1160,10 +1195,9 @@ func search(ctx *ExecContext, query, format string) error { return err } fmt.Fprintln(ctx.Output, string(jsonBytes)) - default: // "table" - return execSearch(ctx, &ast.SearchStmt{Query: query}) } + warnIfSourceIndexMissing(ctx) return nil } diff --git a/mdl/executor/cmd_search_source_index_test.go b/mdl/executor/cmd_search_source_index_test.go new file mode 100644 index 000000000..ec171ca67 --- /dev/null +++ b/mdl/executor/cmd_search_source_index_test.go @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/catalog" +) + +// searchCacheFixture writes a real catalog cache next to a stand-in .mpr, at +// the given build mode, so search's ensureCatalog loads it instead of +// building. sourceRows seeds CATALOG.SOURCE; the strings table always holds +// one caption containing "Invoice". +func searchCacheFixture(t *testing.T, mode string, sourceRows [][2]string) (*ExecContext, *bytes.Buffer, *bytes.Buffer) { + t.Helper() + dir := t.TempDir() + mprPath := filepath.Join(dir, "app.mpr") + if err := os.WriteFile(mprPath, []byte("stand-in"), 0o644); err != nil { + t.Fatal(err) + } + fi, err := os.Stat(mprPath) + if err != nil { + t.Fatal(err) + } + + cat, err := catalog.New() + if err != nil { + t.Fatal(err) + } + db := cat.CatalogDB() + if _, err := db.Exec(`INSERT INTO strings (QualifiedName, ObjectType, StringValue, StringContext, ModuleName) + VALUES ('Sales.Invoice_Overview', 'PAGE', 'Invoice overview', 'caption', 'Sales')`); err != nil { + t.Fatalf("seed strings: %v", err) + } + for _, r := range sourceRows { + if _, err := db.Exec(`INSERT INTO source (QualifiedName, ObjectType, SourceText, ModuleName, ElementId) + VALUES (?, 'MICROFLOW', ?, 'Sales', 'id-1')`, r[0], r[1]); err != nil { + t.Fatalf("seed source: %v", err) + } + } + if err := cat.SetCacheInfo(mprPath, fi.ModTime(), "11.0.0", mode, 0); err != nil { + t.Fatal(err) + } + cachePath := filepath.Join(dir, ".mxcli", "catalog.db") + if err := os.MkdirAll(filepath.Dir(cachePath), 0o755); err != nil { + t.Fatal(err) + } + if err := cat.SaveToFile(cachePath); err != nil { + t.Fatalf("save cache: %v", err) + } + cat.Close() + + var diag bytes.Buffer + ctx, out := newMockCtx(t, withQuiet()) + ctx.MprPath = mprPath + ctx.Diagnostics = &diag + t.Cleanup(func() { + if ctx.Catalog != nil { + ctx.Catalog.Close() + } + }) + return ctx, out, &diag +} + +const sourceIndexHint = "refresh catalog full source" + +// A full-mode catalog has no source index. A search there answered only from +// string literals and said nothing — an agent read the empty source section +// as "no MDL mentions this". The warning must name the command that builds it, +// and go to diagnostics so --format json stays parseable. +func TestSearch_WarnsWhenSourceIndexNotBuilt(t *testing.T) { + for _, format := range []string{"json", "names", "table"} { + t.Run(format, func(t *testing.T) { + ctx, out, diag := searchCacheFixture(t, "full", nil) + if err := search(ctx, "Invoice", format); err != nil { + t.Fatalf("search: %v", err) + } + if !strings.Contains(diag.String(), sourceIndexHint) { + t.Errorf("expected a missing-source-index warning naming %q on diagnostics; got %q", sourceIndexHint, diag.String()) + } + if strings.Contains(out.String(), sourceIndexHint) || strings.Contains(out.String(), "Warning") { + t.Errorf("warning leaked into the results payload:\n%s", out.String()) + } + if format == "json" { + var v []map[string]any + if err := json.Unmarshal(out.Bytes(), &v); err != nil { + t.Errorf("json output is not pure JSON: %v\n%s", err, out.String()) + } + } + }) + } +} + +// The warning is about the index, not about the result: a query that matches +// nothing anywhere on a full-mode catalog must still say the source was not +// searched (this is the case where "No matches found." is most misleading). +func TestSearch_WarnsOnNoMatchesWithoutSourceIndex(t *testing.T) { + ctx, _, diag := searchCacheFixture(t, "full", nil) + if err := execSearch(ctx, &ast.SearchStmt{Query: "Nonexistent"}); err != nil { + t.Fatal(err) + } + if !strings.Contains(diag.String(), sourceIndexHint) { + t.Errorf("expected warning on diagnostics; got %q", diag.String()) + } +} + +// An index that was built but matched nothing is a real answer. Keyed off the +// recorded build mode, not the row count: a source-mode catalog with zero +// source rows must not warn. +func TestSearch_NoWarningWhenSourceIndexBuilt(t *testing.T) { + cases := map[string][][2]string{ + "built-empty": nil, + "built-matching": {{"Sales.ACT_Invoice_Create", "create microflow Sales.ACT_Invoice_Create () begin end;"}}, + } + for name, rows := range cases { + t.Run(name, func(t *testing.T) { + ctx, out, diag := searchCacheFixture(t, "source", rows) + if err := search(ctx, "Invoice", "json"); err != nil { + t.Fatal(err) + } + if diag.Len() != 0 { + t.Errorf("source index is built; expected no warning, got %q", diag.String()) + } + if strings.Contains(out.String(), sourceIndexHint) { + t.Errorf("unexpected hint in output:\n%s", out.String()) + } + }) + } +} diff --git a/mdl/executor/exec_context.go b/mdl/executor/exec_context.go index 7a5e97a65..b6ed7d8d0 100644 --- a/mdl/executor/exec_context.go +++ b/mdl/executor/exec_context.go @@ -34,6 +34,12 @@ type ExecContext struct { // Output is the writer for user-visible output (with line-limit guard). Output io.Writer + // Diagnostics receives warnings about the answer rather than the answer + // itself — "this result is incomplete because…" — so they never land in a + // payload a caller parses (search --format json). Nil means os.Stderr; use + // diagnostics() rather than reading the field. + Diagnostics io.Writer + // describeQualifyAttrs is set by DESCRIBE PAGE while it reads the widgets // inside a data container whose flow cannot be resolved: no entity is in // scope there, so an attribute binding keeps its stored Module.Entity.Attr @@ -135,6 +141,15 @@ type ExecContext struct { ScriptDir string } +// diagnostics returns the writer for warnings about a result (see the +// Diagnostics field): the configured one, or os.Stderr. +func (ctx *ExecContext) diagnostics() io.Writer { + if ctx.Diagnostics != nil { + return ctx.Diagnostics + } + return os.Stderr +} + // ResolveScriptRelative turns a path written inside an MDL script into an // absolute one: relative to the script's own directory when that is known, and // to the working directory otherwise. From a7c2377d3e15349bbc12b85e51f77e7442dbdf1f Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:02:04 +0000 Subject: [PATCH 03/13] fix(structure): show microflow and nanoflow counts at depth 1 `show structure depth 1` (and its JSON form) filtered the catalog on MicroflowType = 'microflow' / 'nanoflow', while the catalog builder stores 'MICROFLOW' / 'NANOFLOW'. SQLite's `=` is case-sensitive, so both counts were always empty, and the summary omits zero counts, so every module appeared to have no flows at all. The builder's values are now exported constants (catalog.MicroflowTypeMicroflow/Nanoflow/Rule), used by the writer, by the structure query and by the linter's DocumentNoun switch, so reader and writer share one spelling. The test runs the real catalog builder over a MockBackend and reads the counts back through structureDepth1 / structureDepth1JSON, so it detects a casing drift between writer and reader; reverting only the reader to lower case makes it fail with the reported symptom. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../bug-tests/structure-flow-counts.mdl | 37 +++++++ mdl/catalog/builder_microflows.go | 21 +++- mdl/executor/cmd_structure.go | 16 ++- .../cmd_structure_flow_counts_test.go | 99 +++++++++++++++++++ mdl/linter/context.go | 4 +- 6 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 mdl-examples/bug-tests/structure-flow-counts.mdl create mode 100644 mdl/executor/cmd_structure_flow_counts_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..48ab98f39 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area": "mdl/executor", "date": "2026-09-26", "symptom": "`show structure depth 1` / `mxcli structure -d 1` never lists microflow or nanoflow counts for any module (Evora Factory Management: FactoryManagement shows entities and pages but not its 127 microflows), in both the text and the JSON form (Microflows/Nanoflows always 0).", "cause": "cmd_structure.go filtered the catalog with MicroflowType = 'microflow' / 'nanoflow'; the catalog builder writes 'MICROFLOW' / 'NANOFLOW'. SQLite `=` on TEXT is case-sensitive, so both counts were empty, and the depth-1 formatter omits zero counts, so absence looked like 'this module has no flows'.", "file": "mdl/executor/cmd_structure.go, mdl/catalog/builder_microflows.go", "insight": "Two silences stacked: queryCountByModule swallows errors and empty results alike, and the summary drops zero counts — so a filter matching nothing produces no signal at any layer. The cheap detector is a test that runs the REAL catalog builder (catalog.NewBuilder over a MockBackend; it needs GetProjectSettingsFunc and ListModuleSettingsFunc set, the mock's defaults panic / error) and reads through the consumer's query — a hand-seeded microflows_data row only proves the reader agrees with the test author. The writer's values are now exported constants (catalog.MicroflowTypeMicroflow/Nanoflow/Rule) and every Go reader uses them; grep for `MicroflowType = '` literals found no other lower-case reader. Same shape as the refs.SourceType drift (#1027): any catalog enum column needs a named vocabulary on the writer side.", "refs": ["fix/structure-flow-counts"]} diff --git a/mdl-examples/bug-tests/structure-flow-counts.mdl b/mdl-examples/bug-tests/structure-flow-counts.mdl new file mode 100644 index 000000000..cfe92d608 --- /dev/null +++ b/mdl-examples/bug-tests/structure-flow-counts.mdl @@ -0,0 +1,37 @@ +-- Bug test: `show structure depth 1` (CLI: `mxcli structure -d 1`) never +-- listed microflow or nanoflow counts, on any project. +-- +-- Found by an agent-orientation audit on a large app (Evora Factory +-- Management): every module line read "N entities, M pages, ..." with no +-- flows at all, although FactoryManagement alone has 127 microflows. +-- +-- Cause: the structure summary filtered the catalog on +-- MicroflowType = 'microflow' / 'nanoflow', while the catalog builder stores +-- 'MICROFLOW' / 'NANOFLOW'. SQLite's `=` is case-sensitive, so both counts +-- were empty and the summary omits zero counts - no error, just absence. +-- +-- Expected after the fix, for module StructFlowCounts: +-- 2 microflows, 1 nanoflow +-- (the rule-free module keeps the check simple; rules are a separate doctype +-- and are never counted as microflows). + +create module StructFlowCounts; + +create microflow StructFlowCounts.ACT_One () +begin + log info 'one'; +end; +/ + +create microflow StructFlowCounts.ACT_Two () +begin + log info 'two'; +end; +/ + +create nanoflow StructFlowCounts.NF_One () +begin +end; +/ + +show structure depth 1 in StructFlowCounts; diff --git a/mdl/catalog/builder_microflows.go b/mdl/catalog/builder_microflows.go index 1bbc97022..b38c42416 100644 --- a/mdl/catalog/builder_microflows.go +++ b/mdl/catalog/builder_microflows.go @@ -10,6 +10,21 @@ import ( "github.com/mendixlabs/mxcli/sdk/microflows" ) +// Values of microflows.MicroflowType — which of the three flow flavours sharing +// microflows_data a row is. Upper-case, like every other catalog type +// vocabulary (refs.SourceType, objects.ObjectType). +// +// Named because readers outside this package filter on them: `show structure` +// compared against 'microflow' / 'nanoflow' for as long as the column has held +// upper case, matched nothing, and so never showed a flow count. SQLite's `=` +// is case-sensitive; a literal that disagrees with the writer fails silently +// as an empty result, never as an error. Filter on these, not on a literal. +const ( + MicroflowTypeMicroflow = "MICROFLOW" + MicroflowTypeNanoflow = "NANOFLOW" + MicroflowTypeRule = "RULE" +) + func (b *Builder) buildMicroflows() error { // Get all microflows (cached — avoids re-parsing in later phases) mfs, err := b.cachedMicroflows() @@ -132,7 +147,7 @@ func (b *Builder) buildMicroflows() error { qualifiedName, moduleName, b.hierarchy.buildFolderPath(mf.ContainerID), // real folder path (Bug 12b class) - "MICROFLOW", + MicroflowTypeMicroflow, mf.Documentation, returnType, len(mf.Parameters), @@ -235,7 +250,7 @@ func (b *Builder) buildMicroflows() error { qualifiedName, moduleName, b.hierarchy.buildFolderPath(nf.ContainerID), // real folder path (Bug 12b class) - "NANOFLOW", + MicroflowTypeNanoflow, nf.Documentation, returnType, len(nf.Parameters), @@ -335,7 +350,7 @@ func (b *Builder) buildMicroflows() error { qualifiedName, moduleName, b.hierarchy.buildFolderPath(rule.ContainerID), - "RULE", + MicroflowTypeRule, rule.Documentation, returnType, len(rule.Parameters), diff --git a/mdl/executor/cmd_structure.go b/mdl/executor/cmd_structure.go index 5cf7f605c..4501445d8 100644 --- a/mdl/executor/cmd_structure.go +++ b/mdl/executor/cmd_structure.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/catalog" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" @@ -66,8 +67,8 @@ func execShowStructure(ctx *ExecContext, s *ast.ShowStmt) error { // and columns for each element type count. func structureDepth1JSON(ctx *ExecContext, modules []structureModule) error { entityCounts := queryCountByModule(ctx, "entities") - mfCounts := queryCountByModule(ctx, "microflows where MicroflowType = 'microflow'") - nfCounts := queryCountByModule(ctx, "microflows where MicroflowType = 'nanoflow'") + mfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeMicroflow)) + nfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeNanoflow)) pageCounts := queryCountByModule(ctx, "pages") enumCounts := queryCountByModule(ctx, "enumerations") snippetCounts := queryCountByModule(ctx, "snippets") @@ -189,8 +190,8 @@ func asString(v any) string { func structureDepth1(ctx *ExecContext, modules []structureModule) error { // Query counts per module from catalog entityCounts := queryCountByModule(ctx, "entities") - mfCounts := queryCountByModule(ctx, "microflows where MicroflowType = 'microflow'") - nfCounts := queryCountByModule(ctx, "microflows where MicroflowType = 'nanoflow'") + mfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeMicroflow)) + nfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeNanoflow)) pageCounts := queryCountByModule(ctx, "pages") enumCounts := queryCountByModule(ctx, "enumerations") snippetCounts := queryCountByModule(ctx, "snippets") @@ -285,6 +286,13 @@ func queryCountByModule(ctx *ExecContext, tableAndWhere string) map[string]int { return counts } +// flowTypeFilter selects one flow flavour from the microflows table, spelled +// with the catalog builder's own constant: a hand-typed lower-case literal here +// matched no row and hid every microflow and nanoflow count. +func flowTypeFilter(flowType string) string { + return fmt.Sprintf("microflows where MicroflowType = '%s'", flowType) +} + // countByModuleFromBackend counts elements per module using the backend (for types without catalog tables). func countByModuleFromBackend(ctx *ExecContext, kind string) map[string]int { counts := make(map[string]int) diff --git a/mdl/executor/cmd_structure_flow_counts_test.go b/mdl/executor/cmd_structure_flow_counts_test.go new file mode 100644 index 000000000..155a60161 --- /dev/null +++ b/mdl/executor/cmd_structure_flow_counts_test.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/catalog" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// flowCountsCatalog runs the REAL catalog builder over a backend holding two +// microflows, one nanoflow and one rule in module Sales. Seeding microflows_data +// by hand would only prove the reader agrees with whatever value the test typed; +// going through the builder is what detects the writer and the reader spelling +// MicroflowType differently. +func flowCountsCatalog(t *testing.T) *catalog.Catalog { + t.Helper() + const mod = model.ID("mod-sales") + be := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + // The builder dereferences project settings; the mock's nil default + // would panic before any flow is catalogued. + GetProjectSettingsFunc: func() (*model.ProjectSettings, error) { return &model.ProjectSettings{}, nil }, + ListModuleSettingsFunc: func() ([]*types.ModuleSettings, error) { return nil, nil }, + ListModulesFunc: func() ([]*model.Module, error) { + return []*model.Module{{BaseElement: model.BaseElement{ID: mod}, Name: "Sales"}}, nil + }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return []*microflows.Microflow{ + {BaseElement: model.BaseElement{ID: "mf-1"}, ContainerID: mod, Name: "ACT_One"}, + {BaseElement: model.BaseElement{ID: "mf-2"}, ContainerID: mod, Name: "ACT_Two"}, + }, nil + }, + ListNanoflowsFunc: func() ([]*microflows.Nanoflow, error) { + return []*microflows.Nanoflow{ + {BaseElement: model.BaseElement{ID: "nf-1"}, ContainerID: mod, Name: "NF_One"}, + }, nil + }, + ListRulesFunc: func() ([]*microflows.Rule, error) { + return []*microflows.Rule{ + {BaseElement: model.BaseElement{ID: "rule-1"}, ContainerID: mod, Name: "RULE_One"}, + }, nil + }, + } + cat, err := catalog.New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + t.Cleanup(func() { cat.Close() }) + if err := catalog.NewBuilder(cat, be).Build(nil); err != nil { + t.Fatalf("catalog build: %v", err) + } + return cat +} + +// `show structure depth 1` never listed microflow or nanoflow counts: it +// filtered on MicroflowType = 'microflow' / 'nanoflow' while the catalog +// builder stores 'MICROFLOW' / 'NANOFLOW', so both queries matched nothing and +// the zero counts were silently omitted from the summary line. +func TestStructureDepth1CountsFlowsFromBuiltCatalog(t *testing.T) { + cat := flowCountsCatalog(t) + ctx, buf := newMockCtx(t) + ctx.Catalog = cat + + mods := []structureModule{{Name: "Sales", ID: "mod-sales"}} + if err := structureDepth1(ctx, mods); err != nil { + t.Fatalf("structureDepth1: %v", err) + } + out := buf.String() + // The rule is its own doctype: it must not be counted as a microflow. + for _, want := range []string{"2 microflows", "1 nanoflow"} { + if !strings.Contains(out, want) { + t.Errorf("depth-1 summary lacks %q; got:\n%s", want, out) + } + } +} + +func TestStructureDepth1JSONCountsFlowsFromBuiltCatalog(t *testing.T) { + cat := flowCountsCatalog(t) + ctx, buf := newMockCtx(t) + ctx.Catalog = cat + ctx.Format = FormatJSON + + mods := []structureModule{{Name: "Sales", ID: "mod-sales"}} + if err := structureDepth1JSON(ctx, mods); err != nil { + t.Fatalf("structureDepth1JSON: %v", err) + } + out := buf.String() + for _, want := range []string{`"Microflows": 2`, `"Nanoflows": 1`} { + if !strings.Contains(out, want) { + t.Errorf("JSON structure lacks %s; got:\n%s", want, out) + } + } +} diff --git a/mdl/linter/context.go b/mdl/linter/context.go index e27ef4fbf..13b9d05f6 100644 --- a/mdl/linter/context.go +++ b/mdl/linter/context.go @@ -615,9 +615,9 @@ type Microflow struct { // with an imprecise noun than not at all. func (m Microflow) DocumentNoun() string { switch m.MicroflowType { - case "NANOFLOW": + case catalog.MicroflowTypeNanoflow: return "nanoflow" - case "RULE": + case catalog.MicroflowTypeRule: return "rule" default: return "microflow" From 11ff6f204e02f34454cd2590f7dbe1e9930d9020 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:10:34 +0000 Subject: [PATCH 04/13] fix(catalog): reference edges to attributes, enumerations, workflows and mapped entities The refs graph stopped at documents. `impact Module.Entity.Attr` answered "not referenced" for an attribute a microflow writes and a page displays (Evora: DigitalTwin.Machine.NumberOfIncidents), an enumeration had no inbound edge at all, a workflow started only by a microflow had no caller, a page navigating an association and a mapping mapping an entity were invisible. - A raw-document walk over microflows, nanoflows, rules, pages, snippets, workflows and import/export mappings matches every string value against the names the model declares: whole-string matches are structured references (MemberChange.Attribute, AttributeRef.Attribute, EntityRefStep.Association, EnumerationType.Enumeration, ObjectMappingElement.Entity); inside expressions, association paths and qualified enumeration values. New kinds: member, type, value, mapping. - XPath constraints resolve bare attribute names against their target entity (and its generalizations), association paths, and enum attributes compared to a literal (kind xpath). Page/snippet constraints had no target entity because resolveEntityRefFromBSON read a key no stored EntityRef carries. - Entities -> enumerations from attribute types (kind type). - WorkflowCallAction -> WORKFLOW (kind call). - New types ATTRIBUTE, ENUMERATION, ENUMERATION_VALUE, IMPORT_MAPPING, EXPORT_MAPPING published in the lint-rule vocabulary; members kept off the graph_god_nodes asset side; CatalogSchemaVersion 15. Not covered: a bare member named through a variable in a free-text expression ($Order/Total), whose type is not known to the catalog. Co-Authored-By: Claude Opus 5.5 --- .../skills/mendix/write-lint-rules/SKILL.md | 8 +- mdl/catalog/builder.go | 11 +- mdl/catalog/builder_member_refs.go | 516 ++++++++++++++++++ mdl/catalog/builder_member_refs_test.go | 373 +++++++++++++ mdl/catalog/builder_references.go | 32 ++ mdl/catalog/builder_references_test.go | 7 + mdl/catalog/builder_xpath.go | 49 +- mdl/catalog/builder_xpath_test.go | 38 ++ mdl/catalog/catalogdb.go | 4 + mdl/catalog/lint_rule_doc_vocabulary_test.go | 1 + mdl/catalog/tables.go | 15 +- 11 files changed, 1030 insertions(+), 24 deletions(-) create mode 100644 mdl/catalog/builder_member_refs.go create mode 100644 mdl/catalog/builder_member_refs_test.go diff --git a/.claude/skills/mendix/write-lint-rules/SKILL.md b/.claude/skills/mendix/write-lint-rules/SKILL.md index 4f2457759..dbc6f5c92 100644 --- a/.claude/skills/mendix/write-lint-rules/SKILL.md +++ b/.claude/skills/mendix/write-lint-rules/SKILL.md @@ -515,13 +515,13 @@ Returned by `permissions()` (all types) or `permissions_for()` (entity-specific) ### reference | Property | Type | Example | |----------|------|---------| -| `source_type` | string | The document the edge comes FROM, upper-case: `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"SNIPPET"`, `"ENTITY"`, `"ASSOCIATION"`, `"WORKFLOW"`, `"NAVIGATION"`, `"SCHEDULED_EVENT"`, `"PUBLISHED_REST_OPERATION"`, `"PROJECT_SETTINGS"` | +| `source_type` | string | The document the edge comes FROM, upper-case: `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"SNIPPET"`, `"ENTITY"`, `"ASSOCIATION"`, `"WORKFLOW"`, `"NAVIGATION"`, `"SCHEDULED_EVENT"`, `"PUBLISHED_REST_OPERATION"`, `"PROJECT_SETTINGS"`, `"IMPORT_MAPPING"`, `"EXPORT_MAPPING"` | | `source_id` | string | Source UUID | | `source_name` | string | `"Sales.ACT_Customer_Create"` | -| `target_type` | string | What it points AT, upper-case: `"ENTITY"`, `"ASSOCIATION"`, `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"LAYOUT"`, `"WORKFLOW"`, `"WIDGET"`, `"JAVA_ACTION"`, `"REST_OPERATION"`, `"REGULAR_EXPRESSION"`. `LAYOUT` and `WIDGET` are only ever targets; `SCHEDULED_EVENT` and `PROJECT_SETTINGS` only ever sources | +| `target_type` | string | What it points AT, upper-case: `"ENTITY"`, `"ASSOCIATION"`, `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"LAYOUT"`, `"WORKFLOW"`, `"WIDGET"`, `"JAVA_ACTION"`, `"REST_OPERATION"`, `"REGULAR_EXPRESSION"`, `"ATTRIBUTE"`, `"ENUMERATION"`, `"ENUMERATION_VALUE"`. `LAYOUT`, `WIDGET`, `ATTRIBUTE`, `ENUMERATION` and `ENUMERATION_VALUE` are only ever targets; `SCHEDULED_EVENT` and `PROJECT_SETTINGS` only ever sources | | `target_id` | string | Target UUID | -| `target_name` | string | `"Sales.Customer"` | -| `ref_kind` | string | How it references: `"call"`, `"create"`, `"retrieve"`, `"change"`, `"delete"`, `"show_page"`, `"datasource"`, `"action"`, `"layout"`, `"parameter"`, `"return"`, `"generalize"`, `"associate"`, `"home_page"`, `"login_page"`, `"menu_item"`, `"calculate"`, `"schedule"`, `"validate"`, `"settings"`, `"widget"`, `"sync"`, `"publish"`, `"event"` — lower-case, unlike the types above | +| `target_name` | string | `"Sales.Customer"`; three-part for an attribute or an enumeration value: `"Sales.Order.Total"`, `"Sales.OrderStatus.Open"` | +| `ref_kind` | string | How it references: `"call"`, `"create"`, `"retrieve"`, `"change"`, `"delete"`, `"show_page"`, `"datasource"`, `"action"`, `"layout"`, `"parameter"`, `"return"`, `"generalize"`, `"associate"`, `"home_page"`, `"login_page"`, `"menu_item"`, `"calculate"`, `"schedule"`, `"validate"`, `"settings"`, `"widget"`, `"sync"`, `"publish"`, `"event"`, `"member"` (binds/reads/writes an attribute or navigates an association), `"xpath"` (an XPath constraint names it), `"type"` (typed as an enumeration), `"value"` (an expression names an enumeration value), `"mapping"` (an import/export mapping maps the entity) — lower-case, unlike the types above. Attribute names used only through a variable in a free-text expression (`$Order/Total`) have no edge | | `module_name` | string | Source module | ### project_security diff --git a/mdl/catalog/builder.go b/mdl/catalog/builder.go index 7e1ee6f6f..a6ff64162 100644 --- a/mdl/catalog/builder.go +++ b/mdl/catalog/builder.go @@ -580,6 +580,12 @@ func (b *Builder) Build(progress ProgressFunc) error { return fmt.Errorf("failed to build export mappings: %w", err) } + // Build XPath expressions table (full mode only). Before buildReferences, + // which resolves the attributes and associations each constraint names. + if err := b.buildXPathExpressions(); err != nil { + return fmt.Errorf("failed to build xpath expressions: %w", err) + } + // Build cross-references (only in full mode) if err := b.buildReferences(); err != nil { return fmt.Errorf("failed to build references: %w", err) @@ -590,11 +596,6 @@ func (b *Builder) Build(progress ProgressFunc) error { return fmt.Errorf("failed to build permissions: %w", err) } - // Build XPath expressions table (full mode only) - if err := b.buildXPathExpressions(); err != nil { - return fmt.Errorf("failed to build xpath expressions: %w", err) - } - // Build strings FTS table (full mode only) if err := b.buildStrings(); err != nil { return fmt.Errorf("failed to build strings: %w", err) diff --git a/mdl/catalog/builder_member_refs.go b/mdl/catalog/builder_member_refs.go new file mode 100644 index 000000000..fb2398af3 --- /dev/null +++ b/mdl/catalog/builder_member_refs.go @@ -0,0 +1,516 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "database/sql" + "sort" + "strings" + + "go.mongodb.org/mongo-driver/v2/bson" +) + +// Member references: the edges from a document to an ATTRIBUTE, an +// ASSOCIATION it navigates, an ENUMERATION it types something as, or an +// ENUMERATION_VALUE it names. +// +// Until these existed the reference graph stopped at documents, so +// `impact Module.Entity.Attr` answered "not referenced" for an attribute that a +// microflow writes and a page displays (Evora Factory Management: +// DigitalTwin.Machine.NumberOfIncidents). A missing edge is a wrong answer, not +// a missing feature — the tool states it confidently and the caller deletes. +// +// The sites are found by walking the RAW document, not the typed readers. A +// typed walk reaches the sites someone wrote a case for — change members, +// retrieve sorting — and silently misses the rest (aggregate-by-attribute, +// list operations, text template parameters, conditional visibility, pluggable +// widget attribute properties, mapping elements …). Every one of those stores +// the member by its fully qualified name, so matching every string value +// against the set of names the model actually declares reaches all of them with +// no per-type code. The match is on the WHOLE string (or on a whole path token +// inside an expression), against names that exist, so an unrelated string +// cannot produce an edge. +// +// What this cannot see: an attribute named only by its bare name through a +// variable in an expression (`$Order/Total`), because resolving `$Order` needs +// the variable's type. XPath is different — its context entity is known — and +// is handled by xpathRefs below. + +// memberRefSourceTypes maps the unit types whose documents are walked to the +// catalog object type recorded as refs.SourceType. It is a closed list: the +// SourceType vocabulary is documented to lint-rule authors, and walking every +// unit would put values there (PAGE_TEMPLATE, BUILDING_BLOCK) that name +// design-time templates rather than anything that runs. Domain models are +// absent on purpose: an entity's own access rules and indexes name its members, +// and those are the entity describing itself, not another document using it. +var memberRefSourceTypes = map[string]string{ + "Microflows$Microflow": RefObjectMicroflow, + "Microflows$Nanoflow": RefObjectNanoflow, + "Microflows$Rule": RefObjectRule, + "Forms$Page": RefObjectPage, + "Forms$Snippet": RefObjectSnippet, + "Workflows$Workflow": RefObjectWorkflow, + "ImportMappings$ImportMapping": RefObjectImportMapping, + "ExportMappings$ExportMapping": RefObjectExportMapping, +} + +// memberRefSkipKeys are string properties that hold prose or a document's own +// name, never a reference. XPath constraints are skipped too: xpathRefs reads +// them with their context entity, which resolves bare attribute names the raw +// walk cannot. +var memberRefSkipKeys = map[string]bool{ + "Name": true, + "Documentation": true, + "XPathConstraint": true, + "XpathConstraint": true, +} + +// memberRefIndex is the set of names the model declares, which is what makes a +// string a reference rather than text. +type memberRefIndex struct { + attributes map[string]string // "Mod.Entity.Attr" -> enumeration QN, or "" when not an enumeration + associations map[string]bool + enumerations map[string]bool + enumValues map[string]bool // "Mod.Enum.Value" + entities map[string]bool + generalization map[string]string // entity -> the entity it specializes +} + +// loadMemberRefIndex reads the name sets from the tables buildEntities, +// buildAssociations and buildEnumerations filled earlier in the transaction. +func (b *Builder) loadMemberRefIndex() *memberRefIndex { + idx := &memberRefIndex{ + attributes: map[string]string{}, + associations: map[string]bool{}, + enumerations: map[string]bool{}, + enumValues: map[string]bool{}, + entities: map[string]bool{}, + generalization: map[string]string{}, + } + scan := func(query string, fn func(a, b string)) { + rows, err := b.tx.Query(query) + if err != nil { + return + } + defer rows.Close() + for rows.Next() { + var a, c sql.NullString + if rows.Scan(&a, &c) == nil && a.String != "" { + fn(a.String, c.String) + } + } + } + scan(`SELECT EntityQualifiedName || '.' || Name, COALESCE(EnumerationQualifiedName, '') FROM attributes_data`, + func(qn, enum string) { idx.attributes[qn] = enum }) + scan(`SELECT QualifiedName, '' FROM associations_data`, + func(qn, _ string) { idx.associations[qn] = true }) + scan(`SELECT QualifiedName, '' FROM enumerations_data`, + func(qn, _ string) { idx.enumerations[qn] = true }) + scan(`SELECT EnumerationQualifiedName || '.' || Name, '' FROM enumeration_values_data`, + func(qn, _ string) { idx.enumValues[qn] = true }) + scan(`SELECT QualifiedName, COALESCE(Generalization, '') FROM entities_data`, + func(qn, gen string) { + idx.entities[qn] = true + if gen != "" { + idx.generalization[qn] = gen + } + }) + return idx +} + +// resolveAttribute finds the attribute `name` on entity, or on the entity it +// specializes: an XPath over a specialization names inherited attributes +// bare, and the attribute's qualified name is the generalization's. +func (idx *memberRefIndex) resolveAttribute(entity, name string) (string, bool) { + seen := map[string]bool{} + for e := entity; e != "" && !seen[e]; e = idx.generalization[e] { + seen[e] = true + if _, ok := idx.attributes[e+"."+name]; ok { + return e + "." + name, true + } + } + return "", false +} + +// refEdge is one outbound edge found in a document. +type refEdge struct { + TargetType, TargetName, RefKind string +} + +// edgeSet collects edges once each, in a deterministic order. +type edgeSet struct { + seen map[refEdge]bool + edges []refEdge +} + +func (s *edgeSet) add(targetType, targetName, kind string) { + e := refEdge{targetType, targetName, kind} + if s.seen == nil { + s.seen = map[refEdge]bool{} + } + if !s.seen[e] { + s.seen[e] = true + s.edges = append(s.edges, e) + } +} + +func (s *edgeSet) sorted() []refEdge { + sort.Slice(s.edges, func(i, j int) bool { + a, b := s.edges[i], s.edges[j] + if a.TargetType != b.TargetType { + return a.TargetType < b.TargetType + } + if a.TargetName != b.TargetName { + return a.TargetName < b.TargetName + } + return a.RefKind < b.RefKind + }) + return s.edges +} + +// memberRefsInUnit returns the member, enumeration and (for mappings) entity +// edges one stored document makes. +func memberRefsInUnit(contents []byte, sourceType string, idx *memberRefIndex) []refEdge { + var doc bson.D + if err := bson.Unmarshal(contents, &doc); err != nil { + return nil + } + isMapping := sourceType == RefObjectImportMapping || sourceType == RefObjectExportMapping + var set edgeSet + walkBSONStrings(doc, "", func(key, v string) { + if memberRefSkipKeys[key] || v == "" { + return + } + // A whole-string match is a structured reference: MemberChange.Attribute, + // AttributeRef.Attribute, EntityRefStep.Association, EnumerationType.Enumeration, + // ObjectMappingElement.Entity. + switch { + case hasKey(idx.attributes, v): + set.add(RefObjectAttribute, v, RefKindMember) + return + case idx.associations[v]: + set.add(RefObjectAssociation, v, RefKindMember) + return + case idx.enumerations[v]: + set.add(RefObjectEnumeration, v, RefKindType) + return + case idx.enumValues[v]: + set.add(RefObjectEnumerationValue, v, RefKindValue) + return + case isMapping && idx.entities[v]: + set.add(RefObjectEntity, v, RefKindMapping) + return + } + // Otherwise it may be an expression: enumeration values are always + // written qualified, and an association path names its members. + if strings.ContainsAny(v, "./") { + scanPaths(v, "", idx, func(targetType, name string) { + kind := RefKindMember + if targetType == RefObjectEnumerationValue { + kind = RefKindValue + } + set.add(targetType, name, kind) + }) + } + }) + return set.sorted() +} + +func hasKey(m map[string]string, k string) bool { + _, ok := m[k] + return ok +} + +// walkBSONStrings calls fn for every string property value in a decoded +// document, with the property's key. Arrays of strings are visited with the +// array's key. +func walkBSONStrings(v any, key string, fn func(key, value string)) { + switch t := v.(type) { + case string: + fn(key, t) + case bson.D: + for _, e := range t { + walkBSONStrings(e.Value, e.Key, fn) + } + case bson.M: + for k, x := range t { + walkBSONStrings(x, k, fn) + } + case map[string]any: + for k, x := range t { + walkBSONStrings(x, k, fn) + } + case bson.A: + for _, x := range t { + walkBSONStrings(x, key, fn) + } + case []any: + for _, x := range t { + walkBSONStrings(x, key, fn) + } + } +} + +// xpathKeywords are XPath/expression words that can stand where an attribute +// name does and must never resolve to one. +var xpathKeywords = map[string]bool{ + "and": true, "or": true, "not": true, "true": true, "false": true, + "empty": true, "div": true, "mod": true, "if": true, "then": true, "else": true, +} + +// scanPaths finds the members an XPath constraint or an expression names. +// +// context is the entity bare names are resolved against — the retrieved entity +// for an XPath, "" for an expression (where a bare name hangs off a variable +// whose type is not known here). Inside a path, an entity segment becomes the +// context for the next one, and a predicate `[...]` after a path is evaluated +// against the path's last entity, so `Mod.Assoc/Mod.Other[Name = 'x']` resolves +// Name on Mod.Other. +// +// An enumeration attribute compared to a string literal (`Status = 'Open'`) +// names that enumeration value: XPath spells values as their bare name in +// quotes, so this is the only place an XPath reference to a value is visible. +func scanPaths(text, context string, idx *memberRefIndex, emit func(targetType, name string)) { + stack := []string{context} + top := func() string { return stack[len(stack)-1] } + lastPathEntity := "" + pendingEnum := "" // enumeration of the attribute just seen, until a comparison literal or anything else + i := 0 + for i < len(text) { + c := text[i] + switch { + case c == '\'': + // String literal with '' escaping. + j := i + 1 + var lit strings.Builder + for j < len(text) { + if text[j] == '\'' { + if j+1 < len(text) && text[j+1] == '\'' { + lit.WriteByte('\'') + j += 2 + continue + } + break + } + lit.WriteByte(text[j]) + j++ + } + if pendingEnum != "" && idx.enumValues[pendingEnum+"."+lit.String()] { + emit(RefObjectEnumerationValue, pendingEnum+"."+lit.String()) + } + pendingEnum = "" + lastPathEntity = "" + i = j + 1 + case c == '%': + // XPath token such as '%CurrentDateTime%' outside quotes. + j := strings.IndexByte(text[i+1:], '%') + if j < 0 { + return + } + pendingEnum = "" + i += j + 2 + case c == '[': + ctx := top() + if lastPathEntity != "" { + ctx = lastPathEntity + } + stack = append(stack, ctx) + lastPathEntity = "" + pendingEnum = "" + i++ + case c == ']': + if len(stack) > 1 { + stack = stack[:len(stack)-1] + } + lastPathEntity = "" + pendingEnum = "" + i++ + case isIdentChar(c) || c == '$' || c == '.' || c == '/': + j := i + for j < len(text) && (isIdentChar(text[j]) || text[j] == '$' || text[j] == '.' || text[j] == '/') { + j++ + } + chunk := text[i:j] + i = j + // A function name (`contains(`, `toString(`) is not a member. + k := j + for k < len(text) && text[k] == ' ' { + k++ + } + if k < len(text) && text[k] == '(' { + pendingEnum = "" + lastPathEntity = "" + continue + } + pendingEnum, lastPathEntity = resolvePath(chunk, top(), idx, emit) + case c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '=' || c == '!': + i++ + default: + pendingEnum = "" + lastPathEntity = "" + i++ + } + } +} + +// resolvePath resolves one navigation path such as +// `Mod.Assoc/Mod.Entity/Attr`, `$var/Mod.Assoc/Mod.Entity/Attr` or `Attr`, +// emitting the associations, attributes and enumeration values it names. It +// returns the enumeration of an attribute that ends the path (for a following +// `= 'Value'`) and the entity the path ends on (for a following predicate). +func resolvePath(chunk, context string, idx *memberRefIndex, emit func(targetType, name string)) (pendingEnum, endEntity string) { + cur := context + segs := strings.Split(chunk, "/") + for n, seg := range segs { + pendingEnum, endEntity = "", "" + switch { + case seg == "": + cur = "" + case strings.HasPrefix(seg, "$"): + // A variable: its type is not known here, so what hangs off it + // directly cannot be resolved. Qualified segments after it still can. + cur = "" + case idx.associations[seg]: + emit(RefObjectAssociation, seg) + cur = "" + case idx.entities[seg]: + cur = seg + endEntity = seg + case idx.enumValues[seg]: + emit(RefObjectEnumerationValue, seg) + cur = "" + case !strings.Contains(seg, ".") && cur != "" && !xpathKeywords[seg]: + if attr, ok := idx.resolveAttribute(cur, seg); ok { + emit(RefObjectAttribute, attr) + if n == len(segs)-1 { + pendingEnum = idx.attributes[attr] + } + } + cur = "" + default: + cur = "" + } + } + return pendingEnum, endEntity +} + +// extractMemberRefs walks every document of the types in memberRefSourceTypes +// and emits its member, enumeration and mapping-entity edges. +func (b *Builder) extractMemberRefs(stmt *sql.Stmt, idx *memberRefIndex, projectID, snapshotID string) int { + unitTypes := make([]string, 0, len(memberRefSourceTypes)) + for t := range memberRefSourceTypes { + unitTypes = append(unitTypes, t) + } + sort.Strings(unitTypes) + + count := 0 + for _, unitType := range unitTypes { + sourceType := memberRefSourceTypes[unitType] + units, err := b.reader.ListRawUnitsByType(unitType) + if err != nil { + continue + } + for _, u := range units { + // ListRawUnitsByType matches a PREFIX: Forms$Page also returns + // Forms$PageTemplate units, which are not pages. + if u.Type != unitType || len(u.Contents) == 0 { + continue + } + var named struct { + Name string `bson:"Name"` + } + if bson.Unmarshal(u.Contents, &named) != nil || named.Name == "" { + continue + } + moduleName := b.hierarchy.getModuleName(b.hierarchy.findModuleID(u.ContainerID)) + sourceQN := moduleName + "." + named.Name + for _, e := range memberRefsInUnit(u.Contents, sourceType, idx) { + if _, err := stmt.Exec(sourceType, string(u.ID), sourceQN, + e.TargetType, "", e.TargetName, + e.RefKind, moduleName, projectID, snapshotID); err == nil { + count++ + } + } + } + } + return count +} + +// extractEnumerationTypeRefs emits one `type` edge from every entity to each +// enumeration one of its attributes is typed as. Without it an enumeration had +// no inbound edge at all, and `impact` on it answered "not referenced". +func (b *Builder) extractEnumerationTypeRefs(projectID, snapshotID string) int { + res, err := b.tx.Exec( + `INSERT INTO refs (SourceType, SourceId, SourceName, TargetType, TargetId, TargetName, RefKind, ModuleName, ProjectId, SnapshotId) + SELECT DISTINCT ?, COALESCE(EntityId, ''), EntityQualifiedName, ?, '', EnumerationQualifiedName, ?, ModuleName, ?, ? + FROM attributes_data + WHERE EnumerationQualifiedName IS NOT NULL AND EnumerationQualifiedName != ''`, + RefObjectEntity, RefObjectEnumeration, RefKindType, projectID, snapshotID) + if err != nil { + return 0 + } + n, _ := res.RowsAffected() + return int(n) +} + +// xpathSourceTypes maps xpath_expressions_data.DocumentType to refs.SourceType. +// An access rule's constraint is recorded against the domain model with the +// entity as its qualified name; the entity is what depends on the attribute. +var xpathSourceTypes = map[string]string{ + "MICROFLOW": RefObjectMicroflow, + "NANOFLOW": RefObjectNanoflow, + "PAGE": RefObjectPage, + "SNIPPET": RefObjectSnippet, + "DOMAIN_MODEL": RefObjectEntity, +} + +// extractXPathRefs emits `xpath` edges for the attributes, associations and +// enumeration values each recorded XPath constraint names, resolved against the +// constraint's target entity. buildXPathExpressions runs before buildReferences +// so the table is populated. +func (b *Builder) extractXPathRefs(stmt *sql.Stmt, idx *memberRefIndex, projectID, snapshotID string) int { + rows, err := b.tx.Query(`SELECT DocumentType, DocumentId, DocumentQualifiedName, COALESCE(TargetEntity, ''), + XPathExpression, COALESCE(ModuleName, '') FROM xpath_expressions_data ORDER BY Id`) + if err != nil { + return 0 + } + type xp struct{ docType, docID, docQN, target, xpath, module string } + var all []xp + for rows.Next() { + var r xp + if rows.Scan(&r.docType, &r.docID, &r.docQN, &r.target, &r.xpath, &r.module) == nil { + all = append(all, r) + } + } + rows.Close() + + count := 0 + written := map[string]bool{} + for _, r := range all { + sourceType, ok := xpathSourceTypes[r.docType] + if !ok { + continue + } + sourceID := r.docID + if r.docType == "DOMAIN_MODEL" { + sourceID = "" + } + var set edgeSet + scanPaths(r.xpath, r.target, idx, func(targetType, name string) { + set.add(targetType, name, RefKindXPath) + }) + for _, e := range set.sorted() { + key := sourceType + "\x00" + r.docQN + "\x00" + e.TargetType + "\x00" + e.TargetName + if written[key] { + continue + } + written[key] = true + if _, err := stmt.Exec(sourceType, sourceID, r.docQN, + e.TargetType, "", e.TargetName, + e.RefKind, r.module, projectID, snapshotID); err == nil { + count++ + } + } + } + return count +} diff --git a/mdl/catalog/builder_member_refs_test.go b/mdl/catalog/builder_member_refs_test.go new file mode 100644 index 000000000..aa06ffce9 --- /dev/null +++ b/mdl/catalog/builder_member_refs_test.go @@ -0,0 +1,373 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// The reference graph had no ATTRIBUTE, ENUMERATION or ENUMERATION_VALUE +// targets at all, no microflow -> workflow edge, no page -> association edge and +// no mapping -> entity edge. `impact Module.Entity.Attr` therefore answered +// "(no impact - element is not referenced)" for an attribute a microflow writes +// and a page displays — measured on Evora Factory Management, where +// DigitalTwin.Machine.NumberOfIncidents is set by a change activity and bound on +// DigitalTwin.Machine_Details. An agent acting on that answer deletes a live +// attribute. +// +// The fixture runs the two real passes over documents shaped like the stored +// BSON (key names checked against Evora's units): buildXPathExpressions, then +// buildReferences, and reads what landed in refs. + +const memberRefsModuleID = model.ID("mod-shop") + +func memberRefsFixture(t *testing.T) *Catalog { + t.Helper() + + cat, err := New() + if err != nil { + t.Fatalf("new catalog: %v", err) + } + t.Cleanup(func() { cat.Close() }) + + tx, err := cat.db.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + + // The tables buildReferences reads its name sets from. buildEntities, + // buildAssociations and buildEnumerations fill them in a real build. + seed := []string{ + `INSERT INTO entities_data (Id, Name, QualifiedName, ModuleName, Generalization) VALUES + ('e1', 'Order', 'Shop.Order', 'Shop', ''), + ('e2', 'Customer', 'Shop.Customer', 'Shop', ''), + ('e3', 'SpecialOrder', 'Shop.SpecialOrder', 'Shop', 'Shop.Order')`, + `INSERT INTO attributes_data (Id, Name, EntityId, EntityQualifiedName, ModuleName, DataType, EnumerationQualifiedName) VALUES + ('a1', 'Status', 'e1', 'Shop.Order', 'Shop', 'Enumeration', 'Shop.OrderStatus'), + ('a2', 'Total', 'e1', 'Shop.Order', 'Shop', 'Decimal', ''), + ('a3', 'Code', 'e1', 'Shop.Order', 'Shop', 'String', ''), + ('a4', 'Name', 'e2', 'Shop.Customer', 'Shop', 'String', ''), + ('a5', 'Unused', 'e2', 'Shop.Customer', 'Shop', 'String', '')`, + `INSERT INTO associations_data (Id, Name, QualifiedName, ModuleName, FromEntity, ToEntity) VALUES + ('as1', 'Order_Customer', 'Shop.Order_Customer', 'Shop', 'Shop.Order', 'Shop.Customer')`, + `INSERT INTO enumerations_data (Id, Name, QualifiedName, ModuleName) VALUES + ('en1', 'OrderStatus', 'Shop.OrderStatus', 'Shop')`, + `INSERT INTO enumeration_values_data (Id, EnumerationId, EnumerationQualifiedName, ModuleName, Name) VALUES + ('v1', 'en1', 'Shop.OrderStatus', 'Shop', 'Open'), + ('v2', 'en1', 'Shop.OrderStatus', 'Shop', 'Closed')`, + } + for _, s := range seed { + if _, err := tx.Exec(s); err != nil { + t.Fatalf("seed: %v\n%s", err, s) + } + } + + // The typed microflow: a call-workflow activity and two XPath retrieves. + mf := µflows.Microflow{ + BaseElement: model.BaseElement{ID: "mf-1"}, + ContainerID: memberRefsModuleID, + Name: "ACT_Start", + ObjectCollection: µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{ + newAction("act-wf", µflows.WorkflowCallAction{Workflow: "Shop.WF_Approve"}), + newAction("act-r1", µflows.RetrieveAction{Source: µflows.DatabaseRetrieveSource{ + EntityQualifiedName: "Shop.Order", + XPathConstraint: "[Shop.Order_Customer/Shop.Customer/Name = $n and Status = 'Open' and Code != empty]", + }}), + newAction("act-r2", µflows.RetrieveAction{Source: µflows.DatabaseRetrieveSource{ + EntityQualifiedName: "Shop.SpecialOrder", + XPathConstraint: "[Total > 5]", + }}), + }}, + } + + // The same microflow as stored: the member refs are read from the raw + // document, so a site no typed struct models is still covered. + mfRaw := bson.D{ + {Key: "$Type", Value: "Microflows$Microflow"}, + {Key: "Name", Value: "ACT_Start"}, + // Prose is not a reference: a name mentioned here must not become an edge. + {Key: "Documentation", Value: "Shop.Order.Code"}, + {Key: "ObjectCollection", Value: bson.D{ + {Key: "$Type", Value: "Microflows$MicroflowObjectCollection"}, + {Key: "Objects", Value: bson.A{int32(3), + bson.D{ + {Key: "$Type", Value: "Microflows$ActionActivity"}, + {Key: "Action", Value: bson.D{ + {Key: "$Type", Value: "Microflows$ChangeAction"}, + {Key: "Items", Value: bson.A{int32(3), + bson.D{ + {Key: "$Type", Value: "Microflows$MemberChange"}, + {Key: "Association", Value: ""}, + {Key: "Attribute", Value: "Shop.Order.Total"}, + {Key: "Value", Value: "$Total"}, + }, + bson.D{ + {Key: "$Type", Value: "Microflows$MemberChange"}, + {Key: "Association", Value: "Shop.Order_Customer"}, + {Key: "Attribute", Value: ""}, + {Key: "Value", Value: "$Customer"}, + }, + }}, + }}, + }, + bson.D{ + {Key: "$Type", Value: "Microflows$ExclusiveSplit"}, + {Key: "SplitCondition", Value: bson.D{ + {Key: "$Type", Value: "Microflows$ExpressionSplitCondition"}, + {Key: "Expression", Value: "$Order/Status = Shop.OrderStatus.Closed"}, + }}, + }, + bson.D{ + {Key: "$Type", Value: "Microflows$ActionActivity"}, + {Key: "Action", Value: bson.D{ + {Key: "$Type", Value: "Microflows$CreateVariableAction"}, + {Key: "VariableType", Value: bson.D{ + {Key: "$Type", Value: "DataTypes$EnumerationType"}, + {Key: "Enumeration", Value: "Shop.OrderStatus"}, + }}, + }}, + }, + }}, + }}, + } + + // A page binding an attribute in a text template and navigating an + // association in a data source — neither is a widget's own AttributeRef. + pageRaw := bson.D{ + {Key: "$Type", Value: "Forms$Page"}, + {Key: "Name", Value: "Order_Edit"}, + {Key: "Widgets", Value: bson.A{int32(3), + bson.D{ + {Key: "$Type", Value: "Forms$DynamicText"}, + {Key: "Content", Value: bson.D{ + {Key: "$Type", Value: "Forms$ClientTemplate"}, + {Key: "Parameters", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "Forms$ClientTemplateParameter"}, + {Key: "AttributeRef", Value: bson.D{ + {Key: "$Type", Value: "DomainModels$AttributeRef"}, + {Key: "Attribute", Value: "Shop.Order.Code"}, + }}, + }, + }}, + }}, + }, + bson.D{ + {Key: "$Type", Value: "Forms$ListView"}, + {Key: "DataSource", Value: bson.D{ + {Key: "$Type", Value: "Forms$AssociationSource"}, + {Key: "EntityRef", Value: bson.D{ + {Key: "$Type", Value: "DomainModels$IndirectEntityRef"}, + {Key: "Steps", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "DomainModels$EntityRefStep"}, + {Key: "Association", Value: "Shop.Order_Customer"}, + {Key: "DestinationEntity", Value: "Shop.Customer"}, + }, + }}, + }}, + }}, + }, + }}, + } + + mappingRaw := bson.D{ + {Key: "$Type", Value: "ImportMappings$ImportMapping"}, + {Key: "Name", Value: "IMM_Order"}, + {Key: "Elements", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "ImportMappings$ObjectMappingElement"}, + {Key: "Entity", Value: "Shop.Order"}, + {Key: "Children", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "ImportMappings$ValueMappingElement"}, + {Key: "Attribute", Value: "Shop.Order.Total"}, + }, + }}, + }, + }}, + } + + rawUnit := func(id, typ string, doc bson.D) *types.RawUnit { + b, err := bson.Marshal(doc) + if err != nil { + t.Fatalf("marshal %s: %v", id, err) + } + return &types.RawUnit{ID: model.ID(id), ContainerID: memberRefsModuleID, Type: typ, Contents: b} + } + units := []*types.RawUnit{ + rawUnit("mf-1", "Microflows$Microflow", mfRaw), + rawUnit("pg-1", "Forms$Page", pageRaw), + rawUnit("im-1", "ImportMappings$ImportMapping", mappingRaw), + } + + b := &Builder{ + catalog: cat, + reader: &mock.MockBackend{ + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return []*microflows.Microflow{mf}, nil + }, + ListRawUnitsByTypeFunc: func(prefix string) ([]*types.RawUnit, error) { + var out []*types.RawUnit + for _, u := range units { + if prefix == "" || u.Type == prefix || len(u.Type) >= len(prefix) && u.Type[:len(prefix)] == prefix { + out = append(out, u) + } + } + return out, nil + }, + GetNavigationFunc: func() (*types.NavigationDocument, error) { + return &types.NavigationDocument{}, nil + }, + }, + snapshot: &Snapshot{ID: "snap-1"}, + hierarchy: &hierarchy{ + moduleIDs: map[model.ID]bool{memberRefsModuleID: true}, + moduleNames: map[model.ID]string{memberRefsModuleID: "Shop"}, + containerParent: map[model.ID]model.ID{}, + folderNames: map[model.ID]string{}, + }, + tx: tx, + fullMode: true, + } + + if err := b.buildXPathExpressions(); err != nil { + t.Fatalf("buildXPathExpressions: %v", err) + } + if err := b.buildReferences(); err != nil { + t.Fatalf("buildReferences: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + return cat +} + +type refRow struct{ sourceType, sourceName, targetType, targetName, kind string } + +func refsTo(t *testing.T, cat *Catalog, target string) map[refRow]int { + t.Helper() + rows, err := cat.db.Query(`SELECT SourceType, SourceName, TargetType, TargetName, RefKind FROM refs WHERE TargetName = ?`, target) + if err != nil { + t.Fatalf("query refs: %v", err) + } + defer rows.Close() + got := map[refRow]int{} + for rows.Next() { + var r refRow + if err := rows.Scan(&r.sourceType, &r.sourceName, &r.targetType, &r.targetName, &r.kind); err != nil { + t.Fatalf("scan: %v", err) + } + got[r]++ + } + return got +} + +func TestReferencesReachMembersEnumerationsWorkflowsAndMappings(t *testing.T) { + cat := memberRefsFixture(t) + + want := []refRow{ + // microflow -> workflow: the call-workflow activity. + {"MICROFLOW", "Shop.ACT_Start", "WORKFLOW", "Shop.WF_Approve", "call"}, + // Attributes, from every kind of site. + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Order.Total", "member"}, // change activity + {"PAGE", "Shop.Order_Edit", "ATTRIBUTE", "Shop.Order.Code", "member"}, // text template parameter + {"IMPORT_MAPPING", "Shop.IMM_Order", "ATTRIBUTE", "Shop.Order.Total", "member"}, // value mapping element + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Customer.Name", "xpath"}, // path segment after an entity + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Order.Status", "xpath"}, // bare name on the retrieved entity + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Order.Code", "xpath"}, // compared to empty + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Order.Total", "xpath"}, // inherited: retrieve of the specialization + {"MICROFLOW", "Shop.ACT_Start", "ASSOCIATION", "Shop.Order_Customer", "member"}, // change activity + {"MICROFLOW", "Shop.ACT_Start", "ASSOCIATION", "Shop.Order_Customer", "xpath"}, // xpath path + {"PAGE", "Shop.Order_Edit", "ASSOCIATION", "Shop.Order_Customer", "member"}, // widget -> association + {"IMPORT_MAPPING", "Shop.IMM_Order", "ENTITY", "Shop.Order", "mapping"}, // mapping -> entity + {"ENTITY", "Shop.Order", "ENUMERATION", "Shop.OrderStatus", "type"}, // attribute type + {"MICROFLOW", "Shop.ACT_Start", "ENUMERATION", "Shop.OrderStatus", "type"}, // variable type + {"MICROFLOW", "Shop.ACT_Start", "ENUMERATION_VALUE", "Shop.OrderStatus.Closed", "value"}, // expression + {"MICROFLOW", "Shop.ACT_Start", "ENUMERATION_VALUE", "Shop.OrderStatus.Open", "xpath"}, // enum attribute compared to a literal + } + + for _, w := range want { + got := refsTo(t, cat, w.targetName) + if got[w] == 0 { + t.Errorf("missing edge %s %s -> %s %s (%s); refs to %s: %v", + w.sourceType, w.sourceName, w.targetType, w.targetName, w.kind, w.targetName, got) + } + if got[w] > 1 { + t.Errorf("edge %v emitted %d times; one document using a member twice is one edge", w, got[w]) + } + } + + // Prose in Documentation is not a use, so the only microflow edge to + // Shop.Order.Code is the XPath one. + for r := range refsTo(t, cat, "Shop.Order.Code") { + if r.sourceType == "MICROFLOW" && r.kind != "xpath" { + t.Errorf("documentation text produced an edge: %v", r) + } + } + // An attribute nothing uses stays unreferenced — the control that shows + // the walk matches names rather than emitting every attribute it knows. + if got := refsTo(t, cat, "Shop.Customer.Unused"); len(got) != 0 { + t.Errorf("unused attribute has references: %v", got) + } +} + +// scanPaths is the resolver both XPath constraints and expressions go through. +// Each case pins one way a bare word could be mistaken for a member, or a real +// member missed. +func TestScanPaths(t *testing.T) { + idx := &memberRefIndex{ + attributes: map[string]string{ + "Shop.Order.Status": "Shop.OrderStatus", "Shop.Order.Code": "", + "Shop.Order.empty": "", "Shop.Order.contains": "", "Shop.Customer.Name": "", + }, + associations: map[string]bool{"Shop.Order_Customer": true}, + enumValues: map[string]bool{"Shop.OrderStatus.Open": true}, + entities: map[string]bool{"Shop.Order": true, "Shop.Customer": true}, + generalization: map[string]string{}, + } + cases := []struct { + name, text, context string + want []string + }{ + {"bare attribute on the context entity", "[Code = 'x']", "Shop.Order", []string{"ATTRIBUTE Shop.Order.Code"}}, + {"no context entity resolves nothing bare", "[Code = 'x']", "", nil}, + {"keyword that is also an attribute name", "[Code != empty]", "Shop.Order", []string{"ATTRIBUTE Shop.Order.Code"}}, + {"function name that is also an attribute name", "[contains(Code, 'x')]", "Shop.Order", []string{"ATTRIBUTE Shop.Order.Code"}}, + {"XPath token between percent signs", "[Code = '[%CurrentUser%]' or Code = %CurrentDateTime%]", "Shop.Order", []string{"ATTRIBUTE Shop.Order.Code"}}, + {"predicate after a path is evaluated on the path's entity", + "[Shop.Order_Customer/Shop.Customer[Name = 'x']]", "Shop.Order", + []string{"ASSOCIATION Shop.Order_Customer", "ATTRIBUTE Shop.Customer.Name"}}, + {"enumeration attribute compared to a literal names the value", "[Status = 'Open']", "Shop.Order", + []string{"ATTRIBUTE Shop.Order.Status", "ENUMERATION_VALUE Shop.OrderStatus.Open"}}, + {"a literal not compared to the enum attribute names nothing", "[Status = Code and Code = 'Open']", "Shop.Order", + []string{"ATTRIBUTE Shop.Order.Code", "ATTRIBUTE Shop.Order.Status"}}, + {"expression: association path from a variable", "$o/Shop.Order_Customer/Shop.Customer/Name", "", + []string{"ASSOCIATION Shop.Order_Customer", "ATTRIBUTE Shop.Customer.Name"}}, + {"expression: bare member of a variable is not resolvable", "$o/Code", "", nil}, + {"expression: qualified enumeration value", "if $o/Status = Shop.OrderStatus.Open then 1 else 2", "", + []string{"ENUMERATION_VALUE Shop.OrderStatus.Open"}}, + {"a qualified name inside a string literal is text", "'Shop.OrderStatus.Open'", "", nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var set edgeSet + scanPaths(tc.text, tc.context, idx, func(tt, n string) { set.add(tt, n, "") }) + var got []string + for _, e := range set.sorted() { + got = append(got, e.TargetType+" "+e.TargetName) + } + if strings.Join(got, "|") != strings.Join(tc.want, "|") { + t.Errorf("scanPaths(%q, %q) = %v, want %v", tc.text, tc.context, got, tc.want) + } + }) + } +} diff --git a/mdl/catalog/builder_references.go b/mdl/catalog/builder_references.go index 210ad9a19..2dc17e7cb 100644 --- a/mdl/catalog/builder_references.go +++ b/mdl/catalog/builder_references.go @@ -39,6 +39,11 @@ const ( RefKindSync = "sync" // An offline navigation profile synchronizes an entity RefKindPublish = "publish" // A published REST operation runs a microflow RefKindEvent = "event" // An entity event handler runs a microflow + RefKindMember = "member" // A document binds, reads or writes an attribute, or navigates an association + RefKindXPath = "xpath" // An XPath constraint names an attribute, association or enumeration value + RefKindType = "type" // An attribute, parameter or variable is typed as an enumeration + RefKindValue = "value" // An expression names an enumeration value + RefKindMapping = "mapping" // An import or export mapping maps an entity ) // Object types recorded in refs.SourceType and refs.TargetType — the catalog's @@ -68,6 +73,11 @@ const ( RefObjectRegularExpression = "REGULAR_EXPRESSION" RefObjectScheduledEvent = "SCHEDULED_EVENT" RefObjectProjectSettings = "PROJECT_SETTINGS" + RefObjectAttribute = "ATTRIBUTE" + RefObjectEnumeration = "ENUMERATION" + RefObjectEnumerationValue = "ENUMERATION_VALUE" + RefObjectImportMapping = "IMPORT_MAPPING" + RefObjectExportMapping = "EXPORT_MAPPING" ) // RefSourceObjectTypes is every value that reaches refs.SourceType, and @@ -89,6 +99,8 @@ var ( RefObjectScheduledEvent, RefObjectPublishedRestOperation, RefObjectProjectSettings, + RefObjectImportMapping, + RefObjectExportMapping, } RefTargetObjectTypes = []string{ @@ -104,6 +116,9 @@ var ( RefObjectJavaAction, RefObjectRestOperation, RefObjectRegularExpression, + RefObjectAttribute, + RefObjectEnumeration, + RefObjectEnumerationValue, } ) @@ -183,6 +198,14 @@ func microflowActionRef(action microflows.MicroflowAction) (targetType, targetNa if a.Operation != "" { return RefObjectRestOperation, a.Operation, RefKindCall, true } + case *microflows.WorkflowCallAction: + // A microflow that starts a workflow is that workflow's caller. Without + // this edge a workflow started only from a microflow had no inbound + // reference: `show callers` said "(no callers found)" and `impact` "not + // referenced" (Evora: AltairIntegration.WF_ScheduleTechnicianAppointment). + if a.Workflow != "" { + return RefObjectWorkflow, a.Workflow, RefKindCall, true + } case *microflows.CreateObjectAction: if a.EntityQualifiedName != "" { return RefObjectEntity, a.EntityQualifiedName, RefKindCreate, true @@ -657,6 +680,15 @@ func (b *Builder) buildReferences() error { // by GRAPH_DEAD_ASSETS, whose advice is to delete them (#1126). refCount += b.extractPublishedRestRefs(stmt, projectID, snapshotID) + // Members, enumerations and mapped entities. Without these the graph ended + // at documents, so `impact` on an attribute, an enumeration or a value said + // "not referenced" however much it was used — the answer an agent reads as + // "safe to delete". See builder_member_refs.go. + idx := b.loadMemberRefIndex() + refCount += b.extractMemberRefs(stmt, idx, projectID, snapshotID) + refCount += b.extractXPathRefs(stmt, idx, projectID, snapshotID) + refCount += b.extractEnumerationTypeRefs(projectID, snapshotID) + b.report("References", refCount) return nil } diff --git a/mdl/catalog/builder_references_test.go b/mdl/catalog/builder_references_test.go index 7f6c480ed..f190c2a87 100644 --- a/mdl/catalog/builder_references_test.go +++ b/mdl/catalog/builder_references_test.go @@ -222,6 +222,13 @@ func TestMicroflowActionRef(t *testing.T) { wantOK: true, targetType: "ASSOCIATION", targetName: "M.Order_Customer", refKind: RefKindRetrieve, }, + { + name: "WorkflowCallAction (previously dropped)", + action: µflows.WorkflowCallAction{Workflow: "M.WF_Approve"}, + wantOK: true, + targetType: "WORKFLOW", targetName: "M.WF_Approve", refKind: RefKindCall, + }, + {name: "empty WorkflowCallAction", action: µflows.WorkflowCallAction{}, wantOK: false}, // Actions whose target is a local variable (no resolvable document QN) must // not emit a ref. {name: "ChangeObjectAction has no document ref", action: µflows.ChangeObjectAction{ChangeVariable: "$Order"}, wantOK: false}, diff --git a/mdl/catalog/builder_xpath.go b/mdl/catalog/builder_xpath.go index 07768fdef..2b66cc5db 100644 --- a/mdl/catalog/builder_xpath.go +++ b/mdl/catalog/builder_xpath.go @@ -279,27 +279,50 @@ func scanBSONArray(v any, fn func(map[string]any)) { // resolveEntityRefFromBSON extracts a qualified entity name from a BSON node // that has an EntityRef field (common in data source nodes). +// +// Studio Pro stores a DomainModels$DirectEntityRef with the name under +// `Entity`, and a DomainModels$IndirectEntityRef (a data source over an +// association path) as `Steps` ending on the entity the constraint applies to. +// `QualifiedName` is kept for the synthetic shape older callers pass; no stored +// EntityRef carries it, which is why every page XPath used to be recorded with +// no target entity. func resolveEntityRefFromBSON(raw map[string]any) string { - // Try EntityRef (used by most data sources) - if entityRef, ok := raw["EntityRef"].(map[string]any); ok { - if name, ok := entityRef["QualifiedName"].(string); ok { + ref := bsonAsMap(raw["EntityRef"]) + if ref == nil { + return "" + } + for _, key := range []string{"Entity", "QualifiedName"} { + if name, ok := ref[key].(string); ok && name != "" { return name } } - // Try bson.D format - if entityRef, ok := raw["EntityRef"].(bson.D); ok { - for _, elem := range entityRef { - if elem.Key == "QualifiedName" { - if name, ok := elem.Value.(string); ok { - return name - } - } + last := "" + scanBSONArray(ref["Steps"], func(step map[string]any) { + if dest, ok := step["DestinationEntity"].(string); ok && dest != "" { + last = dest } + }) + return last +} + +// bsonAsMap returns a decoded sub-document as a map, whichever shape the +// decoder produced it in; nil when v is not a document. +func bsonAsMap(v any) map[string]any { + switch d := v.(type) { + case map[string]any: + return d + case bson.M: + return d + case bson.D: + m := make(map[string]any, len(d)) + for _, e := range d { + m[e.Key] = e.Value + } + return m } - return "" + return nil } -// extractBsonIDString extracts a BSON ID as a string from various formats. func extractBsonIDString(v any) string { if v == nil { return "" diff --git a/mdl/catalog/builder_xpath_test.go b/mdl/catalog/builder_xpath_test.go index 49e426e5a..c825007c2 100644 --- a/mdl/catalog/builder_xpath_test.go +++ b/mdl/catalog/builder_xpath_test.go @@ -4,6 +4,8 @@ package catalog import ( "testing" + + "go.mongodb.org/mongo-driver/bson" ) func TestExtractReferencedEntities(t *testing.T) { @@ -93,6 +95,42 @@ func TestResolveEntityRefFromBSON(t *testing.T) { }, "Module.Entity", }, + // The shapes Studio Pro stores, as buildXPathExpressions decodes them + // (the v1 driver, so nested documents are bson.D). Only QualifiedName was read, which no + // stored EntityRef carries, so every page/snippet XPath constraint was + // recorded with an empty TargetEntity (Evora: 52 of 52) and the bare + // attribute names in it could not be resolved. + { + "stored DirectEntityRef", + map[string]any{ + "EntityRef": bson.D{ + {Key: "$Type", Value: "DomainModels$DirectEntityRef"}, + {Key: "Entity", Value: "Module.Entity"}, + }, + }, + "Module.Entity", + }, + { + "stored IndirectEntityRef ends on the last step's entity", + map[string]any{ + "EntityRef": bson.D{ + {Key: "$Type", Value: "DomainModels$IndirectEntityRef"}, + {Key: "Steps", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "DomainModels$EntityRefStep"}, + {Key: "Association", Value: "Module.A_B"}, + {Key: "DestinationEntity", Value: "Module.B"}, + }, + bson.D{ + {Key: "$Type", Value: "DomainModels$EntityRefStep"}, + {Key: "Association", Value: "Module.B_C"}, + {Key: "DestinationEntity", Value: "Module.C"}, + }, + }}, + }, + }, + "Module.C", + }, { "no EntityRef", map[string]any{"Name": "test"}, diff --git a/mdl/catalog/catalogdb.go b/mdl/catalog/catalogdb.go index 61f4b74ad..c47a0ff30 100644 --- a/mdl/catalog/catalogdb.go +++ b/mdl/catalog/catalogdb.go @@ -21,6 +21,10 @@ type CatalogTx interface { Prepare(query string) (*sql.Stmt, error) Exec(query string, args ...any) (sql.Result, error) QueryRow(query string, args ...any) *sql.Row + // Query reads a multi-row result inside the transaction — the member-ref + // pass needs the name sets earlier passes wrote (attributes, associations, + // enumerations) and the XPath constraints buildXPathExpressions recorded. + Query(query string, args ...any) (*sql.Rows, error) Commit() error Rollback() error } diff --git a/mdl/catalog/lint_rule_doc_vocabulary_test.go b/mdl/catalog/lint_rule_doc_vocabulary_test.go index 5e68a44e2..81dc23f5f 100644 --- a/mdl/catalog/lint_rule_doc_vocabulary_test.go +++ b/mdl/catalog/lint_rule_doc_vocabulary_test.go @@ -224,6 +224,7 @@ func TestSkillDocumentsRealRefKinds(t *testing.T) { RefKindMenuItem, RefKindChange, RefKindDelete, RefKindCalculate, RefKindReturn, RefKindSchedule, RefKindValidate, RefKindSettings, RefKindWidget, RefKindSync, RefKindPublish, RefKindEvent, + RefKindMember, RefKindXPath, RefKindType, RefKindValue, RefKindMapping, } { real[k] = true } diff --git a/mdl/catalog/tables.go b/mdl/catalog/tables.go index 82bed23f1..275e8af5c 100644 --- a/mdl/catalog/tables.go +++ b/mdl/catalog/tables.go @@ -7,6 +7,12 @@ package catalog // // History: // +// 15 — refs gains ATTRIBUTE, ENUMERATION and ENUMERATION_VALUE targets (kinds +// member / xpath / type / value), IMPORT_MAPPING / EXPORT_MAPPING sources +// (kind mapping), and the microflow -> workflow `call` edge; graph_god_nodes +// keeps members off the asset side. Same reason as 11 and 13: refs are only +// written by REFRESH CATALOG FULL, so a cached catalog would keep answering +// `impact Mod.Entity.Attr` with "not referenced" for a used attribute. // 14 — import_mappings_data / export_mappings_data: Id is the document's ID // (was an AUTOINCREMENT integer) and Excluded is recorded; source gains // ElementId. Two mappings may share a name when one is excluded, and the @@ -46,7 +52,7 @@ package catalog // SnapshotSource / SourceId / SourceBranch / SourceRevision columns // from every row (issue #576). // 1 — initial flat schema with denormalized snapshot columns on every row. -const CatalogSchemaVersion = "14" +const CatalogSchemaVersion = "15" // MetaSchemaVersion is the catalog_meta key that records the schema version // the cache was built against. @@ -1400,8 +1406,13 @@ func (c *Catalog) createTables() error { -- ModuleName is its own name (the ELSE d.Asset fallback below) and -- whose ObjectType is NULL. A page's OUT-degree still counts the -- widgets it uses, which is a real dependency. + -- + -- ATTRIBUTE and ENUMERATION_VALUE targets are excluded for the + -- same reason: they are members of a document, not documents, and + -- listing them as assets would bury the entity or enumeration + -- they belong to under its own attributes. SELECT TargetName AS Asset, COUNT(*) AS InDeg, 0 AS OutDeg - FROM refs WHERE TargetName != '' AND TargetType != 'WIDGET' GROUP BY TargetName + FROM refs WHERE TargetName != '' AND TargetType NOT IN ('WIDGET', 'ATTRIBUTE', 'ENUMERATION_VALUE') GROUP BY TargetName UNION ALL SELECT SourceName AS Asset, 0 AS InDeg, COUNT(*) AS OutDeg FROM refs WHERE SourceName != '' GROUP BY SourceName From 493932f08e77a45fd7c122a0bc3849c7fa0b0cc9 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:19:37 +0000 Subject: [PATCH 05/13] fix(refs): one row per reference, impact counts elements, and an empty answer says what was checked `refs` and `impact` printed a row per edge, so a microflow with two retrieve activities over an entity appeared twice (Evora: ProductionLine_Reset on DigitalTwin.Machine); the impact summary counted those rows (MICROFLOW: 9 over six microflows) and printed the types in map order, different between runs. - select distinct, with a total order; the summary counts distinct elements per type, in type order, and the footer gives both numbers. - impact/refs on an enumeration include the edges to its values, with a Target column naming which value. - "(no impact - element is not referenced)" is gone. For an attribute or an enumeration value the message lists the sites that were checked and the ones that are not resolved (a member named through a variable in an expression; a decision branch on an enum), and says to run search first. Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/cmd_query.go | 10 +- .../catalog/show-references-impact.md | 26 +++- mdl/executor/cmd_refs_impact_test.go | 143 ++++++++++++++++++ mdl/executor/cmd_search.go | 82 ++++++---- mdl/executor/reference_target.go | 62 ++++++++ 5 files changed, 292 insertions(+), 31 deletions(-) create mode 100644 mdl/executor/cmd_refs_impact_test.go diff --git a/cmd/mxcli/cmd_query.go b/cmd/mxcli/cmd_query.go index d99b7fbf0..e4e0336e8 100644 --- a/cmd/mxcli/cmd_query.go +++ b/cmd/mxcli/cmd_query.go @@ -74,11 +74,14 @@ Examples: var refsCmd = &cobra.Command{ Use: "refs ", Short: "Find references to an element", - Long: `Find all references to the specified element (entity, microflow, page, etc.). + Long: `Find all references to the specified element (entity, microflow, page, etc., +or an attribute Module.Entity.Attribute, an enumeration, or an enumeration +value Module.Enum.Value). Each (source, kind) is listed once. Examples: mxcli refs -p app.mpr Module.Customer mxcli refs -p app.mpr Module.OrderPage + mxcli refs -p app.mpr Module.Customer.Email `, Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { @@ -98,10 +101,15 @@ var impactCmd = &cobra.Command{ Use: "impact ", Short: "Show impact of changing an element", Long: `Analyze the impact of changing an element by showing all elements that reference it. +The summary counts distinct elements. For an enumeration, the uses of its values are +included. When nothing is found for an attribute or an enumeration value, the message +says which usage sites were checked: one named only through a variable in a free-text +expression ($Order/Total) is not resolved, so run 'mxcli search' before deleting. Examples: mxcli impact -p app.mpr Module.Customer mxcli impact -p app.mpr Module.OrderStatus + mxcli impact -p app.mpr Module.Customer.Email `, Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { diff --git a/docs-site/src/reference/catalog/show-references-impact.md b/docs-site/src/reference/catalog/show-references-impact.md index 943275788..501dcdc35 100644 --- a/docs-site/src/reference/catalog/show-references-impact.md +++ b/docs-site/src/reference/catalog/show-references-impact.md @@ -18,10 +18,28 @@ These commands provide different views of cross-reference information for a give **SHOW CONTEXT OF** assembles the surrounding context of an element -- its definition, its callers, callees, and related elements -- suitable for providing to an LLM or for understanding an element in its broader project context. The optional `DEPTH` parameter controls how many levels of related elements to include. +### Attributes, enumerations and enumeration values + +The target may also be an attribute (`Module.Entity.Attribute`), an enumeration, or an enumeration value (`Module.Enum.Value`). The graph records: + +| Kind | Meaning | +|------|---------| +| `member` | a microflow, nanoflow, rule, page, snippet, workflow or import/export mapping binds, reads or writes the attribute, or navigates the association | +| `xpath` | an XPath constraint names the attribute or association, or compares an enumeration attribute with the value | +| `type` | an attribute, parameter or variable is typed as the enumeration | +| `value` | an expression names the enumeration value | +| `mapping` | an import or export mapping maps the entity | + +`IMPACT OF` an enumeration includes the uses of each of its values, with a `Target` column naming which one. + +Results list each (source, kind) once, and the `IMPACT` summary counts distinct elements. + +When nothing is found, the message says what was searched. An attribute named only through a variable in a free-text expression (`$Order/Total`), and an enumeration value used only as a decision branch, are not resolved by the catalog, so an empty result for an attribute or a value tells you to run `SEARCH` before treating it as unused. + ## Parameters **qualified_name** -: The fully qualified name of the element to analyze (e.g., `Module.EntityName`, `Module.MicroflowName`). +: The fully qualified name of the element to analyze (e.g., `Module.EntityName`, `Module.MicroflowName`, `Module.Entity.Attribute`, `Module.Enum.Value`). **n** (CONTEXT only) : The number of levels of related elements to include. Defaults to 1 if not specified. Higher values include more surrounding context but produce more output. @@ -41,6 +59,12 @@ SHOW REFERENCES TO Sales.Customer; SHOW IMPACT OF Sales.Customer; ``` +### Check an attribute before dropping it + +```sql +SHOW IMPACT OF Sales.Order.DiscountCode; +``` + ### Gather context for a microflow ```sql diff --git a/mdl/executor/cmd_refs_impact_test.go b/mdl/executor/cmd_refs_impact_test.go new file mode 100644 index 000000000..aa360bb25 --- /dev/null +++ b/mdl/executor/cmd_refs_impact_test.go @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/catalog" +) + +// Measured on Evora Factory Management, `impact DigitalTwin.Machine` printed +// FactoryManagement.ProductionLine_Reset | retrieve twice (the microflow has two +// retrieve activities, one edge each) and a summary of "MICROFLOW: 9" over six +// distinct microflows, in an order that changed between runs. `refs` repeated +// the same row. The fixture reproduces each shape. + +func seedRefsCatalog(t *testing.T) *ExecContext { + t.Helper() + cat, err := catalog.New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + t.Cleanup(func() { cat.Close() }) + + db := cat.CatalogDB() + seed := []string{ + `INSERT INTO refs (SourceType, SourceId, SourceName, TargetType, TargetId, TargetName, RefKind, ProjectId, SnapshotId) VALUES + ('MICROFLOW', '', 'Shop.ACT_Reset', 'ENTITY', '', 'Shop.Machine', 'retrieve', 'p', 's'), + ('MICROFLOW', '', 'Shop.ACT_Reset', 'ENTITY', '', 'Shop.Machine', 'retrieve', 'p', 's'), + ('MICROFLOW', '', 'Shop.ACT_Reset', 'ENTITY', '', 'Shop.Machine', 'delete', 'p', 's'), + ('MICROFLOW', '', 'Shop.ACT_Count', 'ENTITY', '', 'Shop.Machine', 'retrieve', 'p', 's'), + ('PAGE', '', 'Shop.Machine_Details', 'ENTITY', '', 'Shop.Machine', 'datasource', 'p', 's'), + ('PAGE', '', 'Shop.Machine_Details', 'ENTITY', '', 'Shop.Machine', 'parameter', 'p', 's'), + ('ASSOCIATION', '', 'Shop.Machine_Line', 'ENTITY', '', 'Shop.Machine', 'associate', 'p', 's'), + ('ENTITY', '', 'Shop.Machine', 'ENUMERATION', '', 'Shop.Status', 'type', 'p', 's'), + ('PAGE', '', 'Shop.Machine_Details', 'ENUMERATION_VALUE', '', 'Shop.Status.Critical', 'value', 'p', 's'), + ('MICROFLOW', '', 'Shop.ACT_Reset', 'ENUMERATION_VALUE', '', 'Shop.Status.Critical', 'value', 'p', 's')`, + `INSERT INTO attributes_data (Id, Name, EntityId, EntityQualifiedName, ModuleName, DataType) VALUES + ('a1', 'Unused', 'e1', 'Shop.Machine', 'Shop', 'String')`, + `INSERT INTO enumerations_data (Id, Name, QualifiedName, ModuleName) VALUES + ('en1', 'Status', 'Shop.Status', 'Shop')`, + `INSERT INTO enumeration_values_data (Id, EnumerationId, EnumerationQualifiedName, ModuleName, Name) VALUES + ('v1', 'en1', 'Shop.Status', 'Shop', 'Critical'), + ('v2', 'en1', 'Shop.Status', 'Shop', 'Retired')`, + } + for _, s := range seed { + if _, err := db.Exec(s); err != nil { + t.Fatalf("seed: %v\n%s", err, s) + } + } + ctx, _ := newMockCtx(t) + ctx.Catalog = cat + return ctx +} + +func runRefsCmd(t *testing.T, fn func(*ExecContext, string) error, target string) string { + t.Helper() + ctx := seedRefsCatalog(t) + if err := fn(ctx, target); err != nil { + t.Fatalf("run: %v", err) + } + return ctx.Output.(interface{ String() string }).String() +} + +func rowsMentioning(out, s string) int { + n := 0 + for _, line := range strings.Split(out, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "|") && strings.Contains(line, s) { + n++ + } + } + return n +} + +func TestShowReferencesListsEachEdgeOnce(t *testing.T) { + out := runRefsCmd(t, showReferences, "Shop.Machine") + // ACT_Reset: retrieve + delete — two edges, not three rows. + if n := rowsMentioning(out, "Shop.ACT_Reset"); n != 2 { + t.Errorf("Shop.ACT_Reset appears in %d rows, want 2 (retrieve, delete):\n%s", n, out) + } + if !strings.Contains(out, "Found 6 reference(s)") { + t.Errorf("want 6 distinct references:\n%s", out) + } +} + +func TestShowImpactCountsElementsNotRows(t *testing.T) { + out := runRefsCmd(t, showImpact, "Shop.Machine") + for _, want := range []string{ + " ASSOCIATION: 1\n MICROFLOW: 2\n PAGE: 1\n", // distinct elements, types in a fixed order + "Found 4 affected element(s)", + } { + if !strings.Contains(out, want) { + t.Errorf("impact output missing %q:\n%s", want, out) + } + } + if n := rowsMentioning(out, "Shop.ACT_Reset"); n != 2 { + t.Errorf("Shop.ACT_Reset appears in %d rows, want 2:\n%s", n, out) + } +} + +// The summary was built by ranging over a map, so two runs of the same +// command could print the types in different orders. +func TestShowImpactIsDeterministic(t *testing.T) { + first := runRefsCmd(t, showImpact, "Shop.Machine") + for i := 0; i < 20; i++ { + if again := runRefsCmd(t, showImpact, "Shop.Machine"); again != first { + t.Fatalf("run %d differs:\n%s\n---\n%s", i, first, again) + } + } +} + +// An enumeration is used through its values as much as through its type; an +// impact that listed only the attribute typed as it would miss the page and +// microflow that break when a value is removed. +func TestShowImpactOfEnumerationIncludesItsValues(t *testing.T) { + out := runRefsCmd(t, showImpact, "Shop.Status") + for _, want := range []string{"Shop.Machine", "Shop.Machine_Details", "Shop.ACT_Reset", "Shop.Status.Critical"} { + if !strings.Contains(out, want) { + t.Errorf("enumeration impact missing %q:\n%s", want, out) + } + } +} + +// With no edge, the catalog cannot say an attribute is unused — only that none +// of the sites it resolves uses it. Saying "not referenced" is what makes an +// agent delete a live attribute. +func TestShowImpactOfUnreferencedAttributeSaysWhatWasChecked(t *testing.T) { + out := runRefsCmd(t, showImpact, "Shop.Machine.Unused") + if strings.Contains(out, "is not referenced") { + t.Errorf("claims the attribute is not referenced:\n%s", out) + } + for _, want := range []string{"expression", "search 'Unused'"} { + if !strings.Contains(out, want) { + t.Errorf("no-reference message missing %q:\n%s", want, out) + } + } + + refs := runRefsCmd(t, showReferences, "Shop.Status.Retired") + if !strings.Contains(refs, "search 'Retired'") { + t.Errorf("enumeration value no-reference message should name what was not checked:\n%s", refs) + } +} diff --git a/mdl/executor/cmd_search.go b/mdl/executor/cmd_search.go index 557fd6277..8e2debdd6 100644 --- a/mdl/executor/cmd_search.go +++ b/mdl/executor/cmd_search.go @@ -213,7 +213,15 @@ func execShowReferences(ctx *ExecContext, s *ast.ShowStmt) error { return err } - typed := s.Name.String() + return showReferences(ctx, s.Name.String()) +} + +// showReferences prints the references to typed from the loaded catalog. +// +// One row per distinct (source, kind): a microflow with two retrieve +// activities over the same entity is one retrieve reference, not two rows +// that read as two callers. +func showReferences(ctx *ExecContext, typed string) error { fmt.Fprintf(ctx.Output, "\nReferences to %s\n", typed) // A widget's TargetName is stored SHOUTED (COMBOBOX) while MDL keywords are @@ -222,21 +230,20 @@ func execShowReferences(ctx *ExecContext, s *ast.ShowStmt) error { targetName, loose := resolveReferenceTarget(ctx, typed) reportResolvedTarget(ctx, typed, targetName, loose) - // Find all references to this target - query := ` - select SourceType, SourceName, RefKind - from refs - where TargetName = ? - ORDER by RefKind, SourceType, SourceName - ` + where, viaValues := refTargetWhere(ctx, targetName) + cols, order := "SourceType, SourceName, RefKind", "RefKind, SourceType, SourceName" + if viaValues { + cols, order = cols+", TargetName as Target", order+", Target" + } + query := `select distinct ` + cols + ` from refs where ` + where + ` order by ` + order - result, err := ctx.Catalog.Query(strings.Replace(query, "?", "'"+escapeSQLString(targetName)+"'", 1)) + result, err := ctx.Catalog.Query(query) if err != nil { return mdlerrors.NewBackend("query references", err) } if result.Count == 0 { - fmt.Fprintln(ctx.Output, "(no references found)") + fmt.Fprintln(ctx.Output, noReferencesMessage(ctx, targetName)) return nil } @@ -257,47 +264,64 @@ func execShowImpact(ctx *ExecContext, s *ast.ShowStmt) error { return err } - typed := s.Name.String() + return showImpact(ctx, s.Name.String()) +} + +// showImpact prints the elements that reference typed, from the loaded catalog. +// +// The summary counts ELEMENTS: it used to count rows, so a microflow that both +// retrieves and deletes an entity was two "affected" microflows, and the types +// came out in map order, different from run to run. +func showImpact(ctx *ExecContext, typed string) error { fmt.Fprintf(ctx.Output, "\nImpact analysis for %s\n", typed) targetName, loose := resolveReferenceTarget(ctx, typed) reportResolvedTarget(ctx, typed, targetName, loose) - // Find all direct references to this target - directQuery := ` - select SourceType, SourceName, RefKind - from refs - where TargetName = ? - ORDER by SourceType, SourceName - ` + where, viaValues := refTargetWhere(ctx, targetName) + cols, order := "SourceType, SourceName, RefKind", "SourceType, SourceName, RefKind" + if viaValues { + cols, order = cols+", TargetName as Target", order+", Target" + } + directQuery := `select distinct ` + cols + ` from refs where ` + where + ` order by ` + order - result, err := ctx.Catalog.Query(strings.Replace(directQuery, "?", "'"+escapeSQLString(targetName)+"'", 1)) + result, err := ctx.Catalog.Query(directQuery) if err != nil { return mdlerrors.NewBackend("query impact", err) } if result.Count == 0 { - fmt.Fprintln(ctx.Output, "(no impact - element is not referenced)") + fmt.Fprintln(ctx.Output, noReferencesMessage(ctx, targetName)) return nil } - // Group by type for summary - typeCounts := make(map[string]int) + // Distinct elements per type. Rows are ordered by SourceType, so the types + // come out sorted. + var types []string + perType := map[string]map[string]bool{} + elements := 0 for _, row := range result.Rows { - if len(row) > 0 { - if t, ok := row[0].(string); ok { - typeCounts[t]++ - } + if len(row) < 2 { + continue + } + t, name := fmt.Sprint(row[0]), fmt.Sprint(row[1]) + if perType[t] == nil { + perType[t] = map[string]bool{} + types = append(types, t) + } + if !perType[t][name] { + perType[t][name] = true + elements++ } } fmt.Fprintf(ctx.Output, "\nSummary:\n") - for t, count := range typeCounts { - fmt.Fprintf(ctx.Output, " %s: %d\n", t, count) + for _, t := range types { + fmt.Fprintf(ctx.Output, " %s: %d\n", t, len(perType[t])) } fmt.Fprintln(ctx.Output) - fmt.Fprintf(ctx.Output, "Found %d affected element(s)\n", result.Count) + fmt.Fprintf(ctx.Output, "Found %d affected element(s) (%d reference(s))\n", elements, result.Count) outputCatalogResults(ctx, result) return nil diff --git a/mdl/executor/reference_target.go b/mdl/executor/reference_target.go index a458c3f8f..de9d99cac 100644 --- a/mdl/executor/reference_target.go +++ b/mdl/executor/reference_target.go @@ -66,3 +66,65 @@ func reportResolvedTarget(ctx *ExecContext, typed, resolved string, matchedLoose } fmt.Fprintf(ctx.Output, "(matched %s)\n", strings.TrimSpace(resolved)) } + +// refTargetWhere returns the refs WHERE clause for a target. An enumeration is +// used through its values as well as its type — a page comparing against +// Mod.Enum.Value breaks when the value is removed — so for an enumeration the +// clause also takes the edges to each of its values, and viaValues reports that +// the caller should show which target each row reached. +func refTargetWhere(ctx *ExecContext, target string) (where string, viaValues bool) { + esc := escapeSQLString(target) + where = fmt.Sprintf("TargetName = '%s'", esc) + if catalogHas(ctx, fmt.Sprintf(`select 1 from enumerations where QualifiedName = '%s' limit 1`, esc)) { + // The name is a LIKE prefix, so its underscores (ENUM_Status) must not + // act as wildcards. + where = fmt.Sprintf(`(TargetName = '%s' or (TargetType = 'ENUMERATION_VALUE' and TargetName like '%s.%%' escape '\'))`, + esc, escapeSQLString(escapeSQLLike(target))) + return where, true + } + return where, false +} + +// escapeSQLLike escapes LIKE wildcards; paired with `escape '\'`. +func escapeSQLLike(s string) string { + r := strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`) + return r.Replace(s) +} + +func catalogHas(ctx *ExecContext, query string) bool { + if ctx == nil || ctx.Catalog == nil { + return false + } + res, err := ctx.Catalog.Query(query) + return err == nil && res.Count > 0 +} + +// noReferencesMessage is what refs/impact print when no edge reaches target. +// +// It says what was searched rather than "element is not referenced": the +// reference graph is built from the sites the catalog resolves, and for an +// attribute or an enumeration value some sites are free text it does not +// resolve. "Not referenced" was read — by an agent, reasonably — as "safe to +// delete", on attributes that were in use. +func noReferencesMessage(ctx *ExecContext, target string) string { + esc := escapeSQLString(target) + member := target + if i := strings.LastIndex(target, "."); i >= 0 { + member = target[i+1:] + } + switch { + case catalogHas(ctx, fmt.Sprintf(`select 1 from attributes where EntityQualifiedName || '.' || Name = '%s' limit 1`, esc)): + return fmt.Sprintf("(no references found to attribute %s)\n"+ + "Checked: attribute bindings and member changes in microflows, nanoflows, rules, pages,\n"+ + "snippets, workflows and import/export mappings, and XPath constraints.\n"+ + "Not checked: the attribute named through a variable in an expression ($Object/%s).\n"+ + "Run `search '%s'` before treating it as unused.", target, member, member) + case catalogHas(ctx, fmt.Sprintf(`select 1 from enumeration_values where EnumerationQualifiedName || '.' || Name = '%s' limit 1`, esc)): + return fmt.Sprintf("(no references found to enumeration value %s)\n"+ + "Checked: qualified uses in expressions (%s) and XPath comparisons of an enumeration\n"+ + "attribute with '%s'.\n"+ + "Not checked: decision branches on an enumeration, which store the bare value name.\n"+ + "Run `search '%s'` before treating it as unused.", target, target, member, member) + } + return fmt.Sprintf("(no references found: nothing in the catalog's reference graph points at %s)", target) +} From fd677e3463cf099219f40ebbfaca2fc19cffad1f Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:19:40 +0000 Subject: [PATCH 06/13] fix(context): related entities come from associations, and callers are listed once `context DigitalTwin.Machine` said "Related Entities: (none found)" for an entity with five associations. The section read refs rows whose SOURCE is an entity, but an association edge's source is the ASSOCIATION, so only generalizations could ever match. It now reads both ends from the associations table (present in a fast catalog too), plus the generalization and the specializations. Also: Direct Callers / Shown By / workflow callers list each source once, and the enumeration context reads the same edge set as impact (type and value uses), grouped into entities, flows and pages. Co-Authored-By: Claude Opus 5.5 --- mdl/executor/cmd_context.go | 81 +++++++++++++----------- mdl/executor/cmd_context_refs_test.go | 91 +++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 36 deletions(-) create mode 100644 mdl/executor/cmd_context_refs_test.go diff --git a/mdl/executor/cmd_context.go b/mdl/executor/cmd_context.go index a0ee96522..cb72a618c 100644 --- a/mdl/executor/cmd_context.go +++ b/mdl/executor/cmd_context.go @@ -151,7 +151,7 @@ func assembleMicroflowContext(ctx *ExecContext, out *strings.Builder, name strin // Direct callers out.WriteString("### Direct Callers\n\n") result, err = ctx.Catalog.Query(fmt.Sprintf( - `select SourceName from refs + `select distinct SourceName from refs where TargetName = '%s' and RefKind = 'call' ORDER by SourceName limit 10`, name)) if err == nil && result.Count > 0 { @@ -243,15 +243,29 @@ func assembleEntityContext(ctx *ExecContext, out *strings.Builder, name string, } out.WriteString("\n") - // Related entities (via associations or generalization) + // Related entities: the other end of each association, the entity this one + // specializes, and the entities that specialize it. + // + // This read refs rows whose SOURCE was an entity, but an association edge's + // source is the ASSOCIATION, so only generalizations were ever found and an + // entity with five associations reported "(none found)". The associations + // table has both ends directly, and is there in a fast-mode catalog too. out.WriteString("### Related Entities\n\n") + esc := escapeSQLString(name) result, err = ctx.Catalog.Query(fmt.Sprintf( - `select distinct TargetName, RefKind from refs - where SourceName = '%s' and TargetType = 'ENTITY' - union - select distinct SourceName, RefKind from refs - where TargetName = '%s' and SourceType = 'ENTITY' - ORDER by RefKind, TargetName limit 10`, name, name)) + `select Related, Relation from ( + select ToEntity as Related, 'association ' || QualifiedName || ': ' || FromEntity || ' -> ' || ToEntity as Relation + from associations where FromEntity = '%[1]s' + union + select FromEntity, 'association ' || QualifiedName || ': ' || FromEntity || ' -> ' || ToEntity + from associations where ToEntity = '%[1]s' + union + select Generalization, 'generalization' + from entities where QualifiedName = '%[1]s' and Generalization is not null and Generalization != '' + union + select QualifiedName, 'specialization' + from entities where Generalization = '%[1]s' + ) order by Relation, Related`, esc)) if err == nil && result.Count > 0 { for _, row := range result.Rows { out.WriteString(fmt.Sprintf("- %v (%v)\n", row[0], row[1])) @@ -316,7 +330,7 @@ func assemblePageContext(ctx *ExecContext, out *strings.Builder, name string, de // Microflows that show this page out.WriteString("### Shown By\n\n") result, err = ctx.Catalog.Query(fmt.Sprintf( - `select SourceName from refs + `select distinct SourceName from refs where TargetName = '%s' and RefKind = 'show_page' ORDER by SourceName limit 10`, name)) if err == nil && result.Count > 0 { @@ -341,33 +355,28 @@ func assembleEnumerationContext(ctx *ExecContext, out *strings.Builder, name str } out.WriteString("\n") - // Entities with attributes of this enumeration type - out.WriteString("### Used By Entities\n\n") - result, err = ctx.Catalog.Query(fmt.Sprintf( - `select distinct SourceName from refs - where TargetName = '%s' and SourceType = 'ENTITY' - ORDER by SourceName limit 15`, name)) - if err == nil && result.Count > 0 { - for _, row := range result.Rows { - out.WriteString(fmt.Sprintf("- %v\n", row[0])) - } - } else { - out.WriteString("(none found)\n") - } - out.WriteString("\n") - - // Microflows that use this enumeration - out.WriteString("### Used By Microflows\n\n") - result, err = ctx.Catalog.Query(fmt.Sprintf( - `select distinct SourceName from refs - where TargetName = '%s' and SourceType = 'MICROFLOW' - ORDER by SourceName limit 15`, name)) - if err == nil && result.Count > 0 { - for _, row := range result.Rows { - out.WriteString(fmt.Sprintf("- %v\n", row[0])) + // Uses of the enumeration: its type (attributes, parameters, variables) + // and its values (expressions, XPath comparisons). refTargetWhere takes + // both, the same set `impact` reports. + where, _ := refTargetWhere(ctx, name) + for _, sec := range []struct{ title, types string }{ + {"Used By Entities", "'ENTITY'"}, + {"Used By Microflows", "'MICROFLOW', 'NANOFLOW', 'RULE'"}, + {"Used By Pages", "'PAGE', 'SNIPPET'"}, + } { + out.WriteString("### " + sec.title + "\n\n") + result, err = ctx.Catalog.Query(fmt.Sprintf( + `select distinct SourceName from refs + where %s and SourceType in (%s) + ORDER by SourceName limit 15`, where, sec.types)) + if err == nil && result.Count > 0 { + for _, row := range result.Rows { + out.WriteString(fmt.Sprintf("- %v\n", row[0])) + } + } else { + out.WriteString("(none found)\n") } - } else { - out.WriteString("(none found)\n") + out.WriteString("\n") } } @@ -561,7 +570,7 @@ func assembleWorkflowContext(ctx *ExecContext, out *strings.Builder, name string // Direct callers (what calls this workflow) out.WriteString("### Direct Callers\n\n") result, err = ctx.Catalog.Query(fmt.Sprintf( - `select SourceName, SourceType from refs + `select distinct SourceName, SourceType from refs where TargetName = '%s' ORDER by SourceName limit 15`, name)) if err == nil && result.Count > 0 { diff --git a/mdl/executor/cmd_context_refs_test.go b/mdl/executor/cmd_context_refs_test.go new file mode 100644 index 000000000..16a1ea648 --- /dev/null +++ b/mdl/executor/cmd_context_refs_test.go @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/catalog" +) + +// `context` on an entity with associations said "Related Entities: (none +// found)". The section read refs rows whose SOURCE is an entity, but an +// association edge's source is the ASSOCIATION, so it only ever found +// generalizations. Evora: DigitalTwin.Machine, five associations. +func TestAssembleEntityContextListsAssociatedEntities(t *testing.T) { + cat, err := catalog.New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + defer cat.Close() + db := cat.CatalogDB() + for _, s := range []string{ + `INSERT INTO entities_data (Id, Name, QualifiedName, ModuleName, EntityType, Generalization) VALUES + ('e1', 'Machine', 'Shop.Machine', 'Shop', 'PERSISTENT', ''), + ('e2', 'Line', 'Shop.Line', 'Shop', 'PERSISTENT', ''), + ('e3', 'Incident', 'Shop.Incident', 'Shop', 'PERSISTENT', ''), + ('e4', 'Robot', 'Shop.Robot', 'Shop', 'PERSISTENT', 'Shop.Machine')`, + `INSERT INTO associations_data (Id, Name, QualifiedName, ModuleName, FromEntity, ToEntity) VALUES + ('as1', 'Machine_Line', 'Shop.Machine_Line', 'Shop', 'Shop.Machine', 'Shop.Line'), + ('as2', 'Incident_Machine', 'Shop.Incident_Machine', 'Shop', 'Shop.Incident', 'Shop.Machine')`, + } { + if _, err := db.Exec(s); err != nil { + t.Fatalf("seed: %v", err) + } + } + ctx, _ := newMockCtx(t) + ctx.Catalog = cat + + var out strings.Builder + assembleEntityContext(ctx, &out, "Shop.Machine", 2) + got := out.String() + related := got[strings.Index(got, "### Related Entities"):] + if strings.Contains(related, "(none found)") { + t.Fatalf("entity with two associations reports no related entities:\n%s", related) + } + for _, want := range []string{"Shop.Line", "Shop.Machine_Line", "Shop.Incident", "Shop.Incident_Machine", "Shop.Robot"} { + if !strings.Contains(related, want) { + t.Errorf("Related Entities missing %q:\n%s", want, related) + } + } +} + +// `context` on a microflow listed a caller once per call activity, so a caller +// with three calls to it read as three callers. +func TestAssembleMicroflowContextListsEachCallerOnce(t *testing.T) { + cat, err := catalog.New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + defer cat.Close() + if _, err := cat.CatalogDB().Exec(`INSERT INTO refs (SourceType, SourceId, SourceName, TargetType, TargetId, TargetName, RefKind, ProjectId, SnapshotId) VALUES + ('MICROFLOW', '', 'Shop.Caller', 'MICROFLOW', '', 'Shop.Sub', 'call', 'p', 's'), + ('MICROFLOW', '', 'Shop.Caller', 'MICROFLOW', '', 'Shop.Sub', 'call', 'p', 's'), + ('MICROFLOW', '', 'Shop.Caller', 'MICROFLOW', '', 'Shop.Sub', 'call', 'p', 's')`); err != nil { + t.Fatalf("seed: %v", err) + } + ctx, _ := newMockCtx(t) + ctx.Catalog = cat + + var out strings.Builder + assembleMicroflowContext(ctx, &out, "Shop.Sub", 1) + callers := out.String()[strings.Index(out.String(), "### Direct Callers"):] + if n := strings.Count(callers, "- Shop.Caller"); n != 1 { + t.Errorf("Shop.Caller listed %d times under Direct Callers, want 1:\n%s", n, callers) + } +} + +// The enumeration context read ENTITY and MICROFLOW sources of edges to the +// enumeration's own name, which never existed; its values were not looked at. +func TestAssembleEnumerationContextIncludesValueUses(t *testing.T) { + ctx := seedRefsCatalog(t) + var out strings.Builder + assembleEnumerationContext(ctx, &out, "Shop.Status") + got := out.String() + for _, want := range []string{"- Shop.Machine\n", "- Shop.ACT_Reset\n", "- Shop.Machine_Details\n"} { + if !strings.Contains(got, want) { + t.Errorf("enumeration context missing %q:\n%s", want, got) + } + } +} From 60963003b30bf79658e15aaf4d30efbfa0583584 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:20:39 +0000 Subject: [PATCH 07/13] fix(describe): trim a stored expression's outer whitespace when rendering it Studio Pro stores expressions exactly as typed, and a trailing newline left in the expression editor is common. describe interpolated the stored text verbatim, so `change $X (Status = Mod.Enum.Val` / `);` put the closing paren or semicolon on a line of its own (297 such lines in Evora Factory Management's microflows alone). One helper, describeExpr (TrimSpace; interior newlines kept), now renders every stored expression the describer emits: change/create members, set, change-list values, aggregate/reduce, list operations, call microflow / nanoflow / java / javascript / external action arguments, show page args, log node + template params, show message params, REST/web service/DB query params, while, decision and rule arguments, and page widget Visible/Editable conditions, action arguments, datasource arguments and client template parameters. It replaces the five ad-hoc TrimSuffix/TrimRight calls that each covered one slot. The stored model is untouched. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../describe-expression-trailing-newline.mdl | 49 ++++++++ ...ows_describe_expression_whitespace_test.go | 108 +++++++++++++++++ mdl/executor/cmd_microflows_format_action.go | 111 +++++++++++------- mdl/executor/cmd_pages_describe_datasource.go | 2 +- mdl/executor/cmd_pages_describe_output.go | 15 +-- 6 files changed, 233 insertions(+), 53 deletions(-) create mode 100644 mdl-examples/bug-tests/describe-expression-trailing-newline.mdl create mode 100644 mdl/executor/cmd_microflows_describe_expression_whitespace_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..b797f63d1 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area":"mdl/executor","date":"2026-09-26","symptom":"`describe microflow` puts the closing `)` or `;` of a statement on a line of its own — `change $X (SyncState = MCPClient.ENUM_SyncState.Syncing` / `);`, or a multi-member `create` rendered one `, Member = …` per line with `) commit;` alone at the end. Parses and executes, but reads as broken and makes every agent-facing describe noisier (Evora Factory Management: 297 lines that are only `);`)","cause":"Studio Pro stores an expression exactly as typed and users leave a trailing newline (or space) in the expression editor. The describer interpolated the stored text verbatim at ~40 sites; five of them had grown their own ad-hoc `TrimSuffix(v, \"\\n\")` / `TrimRight(v, \" \\t\\n\\r\")` (return, declare, decision, rule args, web-service timeout) while every neighbouring slot — change/create members, set, call arguments, list operations, log/message params, REST/DB params, widget Visible/Editable, page action args — kept the newline","file":"`mdl/executor/cmd_microflows_format_action.go` (`describeExpr`), `cmd_pages_describe_output.go`, `cmd_pages_describe_datasource.go`","insight":"**Per-site trims are the tell of a missing helper.** Each earlier fix was right for the slot it was reported against and left the class open; one `describeExpr` (TrimSpace — an expression cannot end inside a string literal, so outer whitespace never carries meaning; interior newlines are kept) closes it. Fixtures typed into tests never carry the trailing newline, so only describing a real Studio Pro-authored app surfaces it — grep describe output for lines starting in column 0 with `)` / `, ` / `;`, which the indenting describer never produces itself. **Idempotence consequence, measured on an Evora copy:** the visitor keeps an argument's trailing whitespace, so the old (ugly) output round-tripped byte-stably, while the trimmed output writes the expression back without the newline. ADR-0008's canon compares strings verbatim, so the first re-exec of a describe of such a flow reports `Replaced` and every later one `Unchanged` — a one-time whitespace-only rewrite, a fixpoint, not churn. Control: two describes of the same state differing only in whitespace; old-exec `Unchanged`, new-exec `Replaced` then `Unchanged`","refs":["Evora Factory Management","docs/13-decisions/0008-identity-and-idempotence.md"]} diff --git a/mdl-examples/bug-tests/describe-expression-trailing-newline.mdl b/mdl-examples/bug-tests/describe-expression-trailing-newline.mdl new file mode 100644 index 000000000..aee131519 --- /dev/null +++ b/mdl-examples/bug-tests/describe-expression-trailing-newline.mdl @@ -0,0 +1,49 @@ +-- describe: a stored expression's trailing newline put `)` / `;` on its own line +-- +-- Studio Pro stores an expression exactly as typed, and a newline left at the +-- end of the expression editor is common (Evora Factory Management: every +-- member of one create, dozens of change/call arguments). `describe microflow` +-- interpolated the stored text verbatim: +-- +-- change $Ticket (TicketState = BugTestNewline.TicketStatus.Open +-- ); +-- +-- The statements below store exactly that shape: the visitor keeps an +-- argument's trailing whitespace, so each value below is written with a +-- trailing "\n", as Studio Pro would have written it. +-- +-- Verify after exec: +-- ./bin/mxcli -p app.mpr -c "describe microflow BugTestNewline.ACT_Newline" +-- every `)` and `;` must follow its expression on the same line, e.g. +-- change $Ticket (TicketState = BugTestNewline.TicketStatus.Open); +-- and the multi-line `find` keeps its interior line break. +-- +-- Re-executing that describe output writes the expressions back WITHOUT the +-- newline, which is a real change to the document: the first re-exec reports +-- `Replaced microflow`, every one after it `Unchanged microflow`. + +create module BugTestNewline; + +create enumeration BugTestNewline.TicketStatus (Open 'Open', Closed 'Closed'); + +create persistent entity BugTestNewline.Ticket ( + Title: string(200), + TicketState: enumeration(BugTestNewline.TicketStatus) +); + +create or modify microflow BugTestNewline.ACT_Newline () +begin + $Ticket = create BugTestNewline.Ticket (Title = 'First' +, TicketState = BugTestNewline.TicketStatus.Closed +); + change $Ticket (TicketState = BugTestNewline.TicketStatus.Open +); + $List = create list of BugTestNewline.Ticket; + add $Ticket to $List; + $Found = find($List, $currentObject/Title = 'First' +and $currentObject/TicketState = BugTestNewline.TicketStatus.Open +); + log info node 'BugTest' 'Title {1}' with ({1} = $Ticket/Title +); + return; +end; diff --git a/mdl/executor/cmd_microflows_describe_expression_whitespace_test.go b/mdl/executor/cmd_microflows_describe_expression_whitespace_test.go new file mode 100644 index 000000000..18f86e4ae --- /dev/null +++ b/mdl/executor/cmd_microflows_describe_expression_whitespace_test.go @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// Studio Pro stores an expression exactly as typed, and users routinely leave +// a trailing newline (or space) in the expression editor. `describe` used to +// emit the stored text verbatim, so the closing `)` or `;` landed on a line of +// its own: +// +// change $IteratorSingleMCPTool (SyncState = MCPClient.ENUM_SyncState.Syncing +// ); +// +// The values below are copied from real documents (Evora Factory Management): +// the leading/trailing whitespace is layout, not meaning, and is dropped; +// interior newlines of a multi-line expression are the author's formatting and +// are kept. +func TestDescribeTrimsStoredExpressionWhitespace(t *testing.T) { + e := newTestExecutor() + cases := []struct { + name string + action microflows.MicroflowAction + want string + }{ + { + name: "change member", + action: µflows.ChangeObjectAction{ + ChangeVariable: "IteratorSingleMCPTool", + Changes: []*microflows.MemberChange{ + {AttributeQualifiedName: "MCPClient.SingleMCPTool.SyncState", Value: "MCPClient.ENUM_SyncState.Syncing\n"}, + }, + }, + want: "change $IteratorSingleMCPTool (SyncState = MCPClient.ENUM_SyncState.Syncing);", + }, + { + name: "create members, trailing newline and trailing space", + action: µflows.CreateObjectAction{ + EntityQualifiedName: "DigitalTwin.TechnicianTicket", + OutputVariable: "TechnicianTicket", + InitialMembers: []*microflows.MemberChange{ + {AttributeQualifiedName: "DigitalTwin.TechnicianTicket.Description", Value: "'Ticket for configuration change:' \n"}, + {AttributeQualifiedName: "DigitalTwin.TechnicianTicket.NeedsApproval", Value: "\ntrue\n"}, + }, + }, + want: "$TechnicianTicket = create DigitalTwin.TechnicianTicket (Description = 'Ticket for configuration change:', NeedsApproval = true);", + }, + { + name: "change variable (member path)", + action: µflows.ChangeVariableAction{ + VariableName: "$Order/Total", + Value: "$Total\r\n", + }, + want: "change $Order (Total = $Total);", + }, + { + name: "set variable", + action: µflows.ChangeVariableAction{VariableName: "Count", Value: "$Count + 1\n"}, + want: "set $Count = $Count + 1;", + }, + { + name: "call microflow argument", + action: µflows.MicroflowCallAction{ + MicroflowCall: µflows.MicroflowCall{ + Microflow: "AgentCommons.Version_Validate_BeforeRun", + ParameterMappings: []*microflows.MicroflowCallParameterMapping{ + {Parameter: "AgentCommons.Version_Validate_BeforeRun.Version", Argument: "$PageHelper/AgentCommons.PageHelper_Version/AgentCommons.Version\n"}, + }, + }, + UseReturnVariable: true, + ResultVariableName: "Valid", + }, + want: "$Valid = call microflow AgentCommons.Version_Validate_BeforeRun(Version = $PageHelper/AgentCommons.PageHelper_Version/AgentCommons.Version);", + }, + { + name: "find keeps interior newlines", + action: µflows.ListOperationAction{ + OutputVariable: "Next", + Operation: µflows.FindOperation{ + ListVariable: "List", + Expression: "$currentObject/IsEnabled\nand $currentObject/IsEdited = false\n", + }, + }, + want: "$Next = find($List, $currentObject/IsEnabled\nand $currentObject/IsEdited = false);", + }, + { + name: "log template parameter", + action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'Node'\n", + TemplateParameters: []string{"$Order/Number\n"}, + }, + want: "log info node 'Node' 'Message' with ({1} = $Order/Number);", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := e.formatAction(tc.action, nil, nil) + if got != tc.want { + t.Errorf("got %q\nwant %q", got, tc.want) + } + }) + } +} diff --git a/mdl/executor/cmd_microflows_format_action.go b/mdl/executor/cmd_microflows_format_action.go index 63ddb44be..e8de99f43 100644 --- a/mdl/executor/cmd_microflows_format_action.go +++ b/mdl/executor/cmd_microflows_format_action.go @@ -51,6 +51,23 @@ func refreshModifier(refresh bool) string { return "" } +// describeExpr renders a stored Mendix expression for describe output. +// +// Studio Pro stores an expression exactly as typed, and a trailing newline (or +// space) left in the expression editor is common in real projects. Emitted +// verbatim it puts the closing `)` or `;` of the MDL statement on a line of its +// own. Leading and trailing whitespace is never part of an expression's meaning +// — an expression cannot end inside a string literal — so it is dropped here, +// in the describer, and never in the stored model. Interior newlines are the +// author's formatting of a multi-line expression and are kept. +// +// Every describe site that interpolates a stored expression goes through this +// helper; the ad-hoc TrimSuffix/TrimRight calls it replaced each covered one +// slot and let the neighbouring ones drift. +func describeExpr(v string) string { + return strings.TrimSpace(v) +} + // escapeExpressionValue escapes raw control characters inside string literals // of a Mendix expression value so it can be safely embedded in MDL output. // The lexer's STRING_LITERAL rule forbids raw \r and \n inside single-quoted @@ -105,7 +122,7 @@ func formatActivity( case *microflows.EndEvent: if activity.ReturnValue != "" { - returnVal := strings.TrimSuffix(activity.ReturnValue, "\n") + returnVal := describeExpr(activity.ReturnValue) // Only add $ prefix for bare identifiers (no operators, quotes, or parens) if !strings.HasPrefix(returnVal, "$") && !isMendixKeyword(returnVal) && !isQualifiedEnumLiteral(returnVal) && !strings.ContainsAny(returnVal, "+'\"()") && !isNumericLiteral(returnVal) { @@ -139,7 +156,7 @@ func formatActivity( case *microflows.LoopedActivity: switch ls := activity.LoopSource.(type) { case *microflows.WhileLoopCondition: - return fmt.Sprintf("while %s", ls.WhileExpression) + return fmt.Sprintf("while %s", describeExpr(ls.WhileExpression)) case *microflows.IterableList: iterVar := "Item" listVar := "List" @@ -232,7 +249,7 @@ func formatAction( if a.DataType != nil { varType = formatMicroflowDataType(ctx, a.DataType, entityNames) } - initialValue := strings.TrimSuffix(a.InitialValue, "\n") + initialValue := describeExpr(a.InitialValue) if initialValue == "" { initialValue = "empty" } @@ -254,13 +271,13 @@ func formatAction( if !strings.HasPrefix(objectName, "$") { objectName = "$" + objectName } - return fmt.Sprintf("change %s (%s = %s);", objectName, attrName, a.Value) + return fmt.Sprintf("change %s (%s = %s);", objectName, attrName, describeExpr(a.Value)) } // Simple variable change if strings.HasPrefix(varName, "$") { - return fmt.Sprintf("set %s = %s;", varName, a.Value) + return fmt.Sprintf("set %s = %s;", varName, describeExpr(a.Value)) } - return fmt.Sprintf("set $%s = %s;", varName, a.Value) + return fmt.Sprintf("set $%s = %s;", varName, describeExpr(a.Value)) case *microflows.CreateObjectAction: // Use EntityQualifiedName (BY_NAME_REFERENCE) or fall back to EntityID lookup @@ -313,7 +330,7 @@ func formatAction( memberName = parts[len(parts)-1] } } - members = append(members, fmt.Sprintf("%s = %s", memberName, escapeExpressionValue(m.Value))) + members = append(members, fmt.Sprintf("%s = %s", memberName, escapeExpressionValue(describeExpr(m.Value)))) } return fmt.Sprintf("$%s = create %s (%s)%s%s;", outputVar, entityName, strings.Join(members, ", "), commitModifier(a.Commit), refreshModifier(a.RefreshInClient)) } @@ -343,7 +360,7 @@ func formatAction( memberName = parts[len(parts)-1] } } - members = append(members, fmt.Sprintf("%s = %s", memberName, escapeExpressionValue(m.Value))) + members = append(members, fmt.Sprintf("%s = %s", memberName, escapeExpressionValue(describeExpr(m.Value)))) } return fmt.Sprintf("change $%s (%s)%s%s;", varName, strings.Join(members, ", "), commitModifier(a.Commit), refreshModifier(a.RefreshInClient)) } @@ -385,13 +402,13 @@ func formatAction( varName := a.ChangeVariable switch a.Type { case microflows.ChangeListTypeAdd: - return fmt.Sprintf("add %s to $%s;", a.Value, varName) + return fmt.Sprintf("add %s to $%s;", describeExpr(a.Value), varName) case microflows.ChangeListTypeRemove: - return fmt.Sprintf("remove %s from $%s;", a.Value, varName) + return fmt.Sprintf("remove %s from $%s;", describeExpr(a.Value), varName) case microflows.ChangeListTypeClear: return fmt.Sprintf("clear $%s;", varName) case microflows.ChangeListTypeSet: - return fmt.Sprintf("set $%s = %s;", varName, a.Value) + return fmt.Sprintf("set $%s = %s;", varName, describeExpr(a.Value)) default: return fmt.Sprintf("change list $%s (%s);", varName, a.Type) } @@ -428,17 +445,17 @@ func formatAction( // are required, so they are rendered even when empty rather than dropped — // a reduce that describes without them cannot be executed back (#1004). if a.Function == microflows.AggregateFunctionReduce { - initial := a.ReduceInitialValue + initial := describeExpr(a.ReduceInitialValue) if initial == "" { initial = "empty" } return fmt.Sprintf("$%s = reduce($%s, %s, initial: %s, returns: %s);", - outputVar, a.InputVariable, a.Expression, initial, + outputVar, a.InputVariable, describeExpr(a.Expression), initial, formatMicroflowDataType(ctx, a.ReduceReturnType, entityNames)) } // Expression-based aggregate: SUM($list, $currentObject/Attr + 1) if a.UseExpression && a.Expression != "" { - return fmt.Sprintf("$%s = %s($%s, %s);", outputVar, fn, a.InputVariable, a.Expression) + return fmt.Sprintf("$%s = %s($%s, %s);", outputVar, fn, a.InputVariable, describeExpr(a.Expression)) } // Attribute-based aggregate: SUM($list.Attr) if attrName != "" && a.Function != microflows.AggregateFunctionCount { @@ -562,7 +579,7 @@ func formatAction( } // Node is an expression in Mendix (e.g., 'TEST' or $variable or 'Prefix' + $var) // Output it as-is since it's already stored as an expression - node := a.LogNodeName + node := describeExpr(a.LogNodeName) if node == "" { node = defaultLogNodeExpression } @@ -576,7 +593,7 @@ func formatAction( if len(a.TemplateParameters) > 0 { var params []string for i, expr := range a.TemplateParameters { - params = append(params, fmt.Sprintf("{%d} = %s", i+1, expr)) + params = append(params, fmt.Sprintf("{%d} = %s", i+1, describeExpr(expr))) } withClause = fmt.Sprintf(" with (%s)", strings.Join(params, ", ")) } @@ -599,7 +616,7 @@ func formatAction( if idx := strings.LastIndex(paramName, "."); idx != -1 { paramName = paramName[idx+1:] } - params = append(params, fmt.Sprintf("%s = %s", paramName, pm.Argument)) + params = append(params, fmt.Sprintf("%s = %s", paramName, describeExpr(pm.Argument))) } } @@ -632,7 +649,7 @@ func formatAction( if idx := strings.LastIndex(paramName, "."); idx != -1 { paramName = paramName[idx+1:] } - params = append(params, fmt.Sprintf("%s = %s", paramName, pm.Argument)) + params = append(params, fmt.Sprintf("%s = %s", paramName, describeExpr(pm.Argument))) } } @@ -667,12 +684,12 @@ func formatAction( valueStr = mdlQuote(v.TypedTemplate.Text) } case *microflows.ExpressionBasedCodeActionParameterValue: - valueStr = v.Expression + valueStr = describeExpr(v.Expression) case *microflows.BasicCodeActionParameterValue: if v.Argument == "" { valueStr = "empty" } else { - valueStr = v.Argument + valueStr = describeExpr(v.Argument) } case *microflows.MicroflowParameterValue: if v.Microflow != "" { @@ -713,7 +730,7 @@ func formatAction( var params []string for _, pm := range a.ParameterMappings { - params = append(params, fmt.Sprintf("%s = %s", pm.ParameterName, pm.Argument)) + params = append(params, fmt.Sprintf("%s = %s", pm.ParameterName, describeExpr(pm.Argument))) } paramStr := "" @@ -752,7 +769,7 @@ func formatAction( // Extract just the parameter name from the qualified name parts := strings.Split(pm.Parameter, ".") paramName := parts[len(parts)-1] - params = append(params, fmt.Sprintf("$%s = %s", paramName, pm.Argument)) + params = append(params, fmt.Sprintf("$%s = %s", paramName, describeExpr(pm.Argument))) } // Build the statement @@ -782,7 +799,11 @@ func formatAction( } result := fmt.Sprintf("show message %s type %s", message, msgType) if len(a.TemplateParameters) > 0 { - result += " objects [" + strings.Join(a.TemplateParameters, ", ") + "]" + objs := make([]string, len(a.TemplateParameters)) + for i, p := range a.TemplateParameters { + objs[i] = describeExpr(p) + } + result += " objects [" + strings.Join(objs, ", ") + "]" } // Without this, a describe -> exec round trip turned a BLOCKING message // box into a non-blocking one. The model carried Blocking on both @@ -943,9 +964,9 @@ func formatAction( valueStr = mdlQuote(v.TypedTemplate.Text) } case *microflows.ExpressionBasedCodeActionParameterValue: - valueStr = v.Expression + valueStr = describeExpr(v.Expression) case *microflows.BasicCodeActionParameterValue: - valueStr = v.Argument + valueStr = describeExpr(v.Argument) case *microflows.EntityTypeCodeActionParameterValue: valueStr = v.Entity } @@ -1038,7 +1059,7 @@ func formatWebServiceCallAction(ctx *ExecContext, a *microflows.WebServiceCallAc parts = append(parts, "receive mapping "+formatWebServiceReference(string(a.ReceiveMappingID))) } if a.TimeoutExpression != "" { - parts = append(parts, "timeout "+strings.TrimRight(a.TimeoutExpression, " \t\n\r")) + parts = append(parts, "timeout "+describeExpr(a.TimeoutExpression)) } return strings.Join(parts, "\n") + ";" } @@ -1059,7 +1080,7 @@ func formatWebServiceArguments(args []microflows.WebServiceArgument) string { if arg.Name == "" { return "" } - parts = append(parts, arg.Name+" = "+arg.Expression) + parts = append(parts, arg.Name+" = "+describeExpr(arg.Expression)) } return strings.Join(parts, ", ") } @@ -1122,9 +1143,9 @@ func formatListOperation(ctx *ExecContext, op microflows.ListOperation, outputVa case *microflows.TailOperation: return fmt.Sprintf("$%s = tail($%s);", outputVar, o.ListVariable) case *microflows.FindOperation: - return fmt.Sprintf("$%s = find($%s, %s);", outputVar, o.ListVariable, o.Expression) + return fmt.Sprintf("$%s = find($%s, %s);", outputVar, o.ListVariable, describeExpr(o.Expression)) case *microflows.FilterOperation: - return fmt.Sprintf("$%s = filter($%s, %s);", outputVar, o.ListVariable, o.Expression) + return fmt.Sprintf("$%s = filter($%s, %s);", outputVar, o.ListVariable, describeExpr(o.Expression)) case *microflows.SortOperation: if len(o.Sorting) > 0 { var sortCols []string @@ -1162,26 +1183,26 @@ func formatListOperation(ctx *ExecContext, op microflows.ListOperation, outputVa case *microflows.FindByAttributeOperation: fieldName := extractFieldName(o.Attribute, o.Association) if fieldName != "" && o.Expression != "" { - return fmt.Sprintf("$%s = find($%s, %s = %s);", outputVar, o.ListVariable, fieldName, o.Expression) + return fmt.Sprintf("$%s = find($%s, %s = %s);", outputVar, o.ListVariable, fieldName, describeExpr(o.Expression)) } else if o.Expression != "" { - return fmt.Sprintf("$%s = find($%s, %s);", outputVar, o.ListVariable, o.Expression) + return fmt.Sprintf("$%s = find($%s, %s);", outputVar, o.ListVariable, describeExpr(o.Expression)) } return fmt.Sprintf("-- $%s = find($%s) — missing attribute/expression", outputVar, o.ListVariable) case *microflows.FilterByAttributeOperation: fieldName := extractFieldName(o.Attribute, o.Association) if fieldName != "" && o.Expression != "" { - return fmt.Sprintf("$%s = filter($%s, %s = %s);", outputVar, o.ListVariable, fieldName, o.Expression) + return fmt.Sprintf("$%s = filter($%s, %s = %s);", outputVar, o.ListVariable, fieldName, describeExpr(o.Expression)) } else if o.Expression != "" { - return fmt.Sprintf("$%s = filter($%s, %s);", outputVar, o.ListVariable, o.Expression) + return fmt.Sprintf("$%s = filter($%s, %s);", outputVar, o.ListVariable, describeExpr(o.Expression)) } return fmt.Sprintf("-- $%s = filter($%s) — missing attribute/expression", outputVar, o.ListVariable) case *microflows.ListRangeOperation: if o.OffsetExpression != "" && o.LimitExpression != "" { - return fmt.Sprintf("$%s = range($%s, %s, %s);", outputVar, o.ListVariable, o.OffsetExpression, o.LimitExpression) + return fmt.Sprintf("$%s = range($%s, %s, %s);", outputVar, o.ListVariable, describeExpr(o.OffsetExpression), describeExpr(o.LimitExpression)) } else if o.OffsetExpression != "" { - return fmt.Sprintf("$%s = range($%s, %s);", outputVar, o.ListVariable, o.OffsetExpression) + return fmt.Sprintf("$%s = range($%s, %s);", outputVar, o.ListVariable, describeExpr(o.OffsetExpression)) } else if o.LimitExpression != "" { - return fmt.Sprintf("$%s = range($%s, 0, %s);", outputVar, o.ListVariable, o.LimitExpression) + return fmt.Sprintf("$%s = range($%s, 0, %s);", outputVar, o.ListVariable, describeExpr(o.LimitExpression)) } return fmt.Sprintf("$%s = range($%s);", outputVar, o.ListVariable) default: @@ -1277,7 +1298,7 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string if i > 0 { sb.WriteString(", ") } - sb.WriteString(fmt.Sprintf("{%d} = %s", i+1, param)) + sb.WriteString(fmt.Sprintf("{%d} = %s", i+1, describeExpr(param))) } sb.WriteString(")") } @@ -1288,7 +1309,7 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string sb.WriteString("\n header ") sb.WriteString(mdlQuote(h.Name)) sb.WriteString(" = ") - sb.WriteString(h.Value) + sb.WriteString(describeExpr(h.Value)) } } @@ -1314,7 +1335,7 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string if i > 0 { sb.WriteString(", ") } - sb.WriteString(fmt.Sprintf("{%d} = %s", i+1, param)) + sb.WriteString(fmt.Sprintf("{%d} = %s", i+1, describeExpr(param))) } sb.WriteString(")") } @@ -1417,14 +1438,14 @@ func formatRestOperationCallAction(ctx *ExecContext, a *microflows.RestOperation if idx := strings.LastIndex(name, "."); idx >= 0 { name = name[idx+1:] } - allParams = append(allParams, struct{ name, value string }{name, pm.Value}) + allParams = append(allParams, struct{ name, value string }{name, describeExpr(pm.Value)}) } for _, qm := range a.QueryParameterMappings { name := qm.Parameter if idx := strings.LastIndex(name, "."); idx >= 0 { name = name[idx+1:] } - allParams = append(allParams, struct{ name, value string }{name, qm.Value}) + allParams = append(allParams, struct{ name, value string }{name, describeExpr(qm.Value)}) } if len(allParams) > 0 { sb.WriteString("\n with (") @@ -1475,7 +1496,7 @@ func formatExecuteDatabaseQueryAction(ctx *ExecContext, a *microflows.ExecuteDat if i > 0 { sb.WriteString(", ") } - sb.WriteString(fmt.Sprintf("%s = %s", pm.ParameterName, pm.Value)) + sb.WriteString(fmt.Sprintf("%s = %s", pm.ParameterName, describeExpr(pm.Value))) } sb.WriteString(")") } @@ -1487,7 +1508,7 @@ func formatExecuteDatabaseQueryAction(ctx *ExecContext, a *microflows.ExecuteDat if i > 0 { sb.WriteString(", ") } - sb.WriteString(fmt.Sprintf("%s = %s", cm.ParameterName, cm.Value)) + sb.WriteString(fmt.Sprintf("%s = %s", cm.ParameterName, describeExpr(cm.Value))) } sb.WriteString(")") } @@ -1846,7 +1867,7 @@ func (e *Executor) formatRestCallAction(a *microflows.RestCallAction) string { func formatSplitCondition(cond microflows.SplitCondition) string { switch c := cond.(type) { case *microflows.ExpressionSplitCondition: - expr := strings.TrimRight(c.Expression, " \t\n\r") + expr := describeExpr(c.Expression) if expr == "" { return "true" } @@ -1862,7 +1883,7 @@ func formatSplitCondition(cond microflows.SplitCondition) string { if idx := strings.LastIndex(paramName, "."); idx >= 0 { paramName = paramName[idx+1:] } - arg := strings.TrimRight(pm.Argument, " \t\n\r") + arg := describeExpr(pm.Argument) if paramName != "" { args = append(args, fmt.Sprintf("%s = %s", paramName, arg)) } else { diff --git a/mdl/executor/cmd_pages_describe_datasource.go b/mdl/executor/cmd_pages_describe_datasource.go index a844e05e4..63bb53f33 100644 --- a/mdl/executor/cmd_pages_describe_datasource.go +++ b/mdl/executor/cmd_pages_describe_datasource.go @@ -489,7 +489,7 @@ func flowSourceArgs(ds map[string]any, settingsKey, flowName string) []rawDataSo if name == "" { continue } - value := extractString(mapping["Expression"]) + value := describeExpr(extractString(mapping["Expression"])) if value == "" { value = pageVariableArgValue(mapping["Variable"]) } diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index 4ee83ba43..76a2ff26f 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -119,13 +119,13 @@ func appendDataGridPagingProps(props []string, w rawWidget) []string { // appendConditionalProps appends VISIBLE IF and EDITABLE IF if present. func appendConditionalProps(props []string, w rawWidget) []string { if w.VisibleIf != "" { - props = append(props, fmt.Sprintf("Visible: [%s]", w.VisibleIf)) + props = append(props, fmt.Sprintf("Visible: [%s]", describeExpr(w.VisibleIf))) } if prop := visibleWhenProp(w); prop != "" { props = append(props, prop) } if w.EditableIf != "" { - props = append(props, fmt.Sprintf("Editable: [%s]", w.EditableIf)) + props = append(props, fmt.Sprintf("Editable: [%s]", describeExpr(w.EditableIf))) } return props } @@ -187,13 +187,13 @@ func appendAppearanceProps(props []string, w rawWidget) []string { props = append(props, formatDesignPropertiesMDL(w.DesignProperties)) } if w.VisibleIf != "" { - props = append(props, fmt.Sprintf("Visible: [%s]", w.VisibleIf)) + props = append(props, fmt.Sprintf("Visible: [%s]", describeExpr(w.VisibleIf))) } if prop := visibleWhenProp(w); prop != "" { props = append(props, prop) } if w.EditableIf != "" { - props = append(props, fmt.Sprintf("Editable: [%s]", w.EditableIf)) + props = append(props, fmt.Sprintf("Editable: [%s]", describeExpr(w.EditableIf))) } return props } @@ -1632,7 +1632,7 @@ func extractPageParameters(ctx *ExecContext, settings map[string]any) string { // Check for Argument (variable reference or expression stored as string) if value == "" { if arg := extractString(mappingMap["Argument"]); arg != "" { - value = arg // e.g., "$Product" or an expression + value = describeExpr(arg) // e.g., "$Product" or an expression } } @@ -1689,7 +1689,7 @@ func extractMicroflowParameters(ctx *ExecContext, settings map[string]any) strin // Check for Expression (used in Pages$MicroflowParameterMapping) if value == "" { if expr := extractString(mappingMap["Expression"]); expr != "" { - value = expr // e.g., "$Product" or an expression + value = describeExpr(expr) // e.g., "$Product" or an expression } } @@ -1748,7 +1748,7 @@ func extractNanoflowParameters(ctx *ExecContext, action map[string]any) string { // Check for Expression (used in Pages$NanoflowParameterMapping) if value == "" { if expr := extractString(mappingMap["Expression"]); expr != "" { - value = expr // e.g., "$Product" or an expression + value = describeExpr(expr) // e.g., "$Product" or an expression } } @@ -1823,6 +1823,7 @@ func extractClientTemplateParameters(ctx *ExecContext, w map[string]any, fieldNa suffixes = append(suffixes, formatParamFormatSuffix(pMap)) // Check for Expression first (literal value) if expr, ok := pMap["Expression"].(string); ok && expr != "" { + expr = describeExpr(expr) // A non-String attribute binding (Integer/DateTime/…) is written as // `toString($currentObject/Attr)` / `toString($param/Attr)` — see // resolveTemplateAttributePathFull. Emit it back as the bare attribute From b70bcdb4067568f892d317b149b622af03ef481c Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:25:09 +0000 Subject: [PATCH 08/13] test(refs): MDL repro for the reference-graph gaps, and findings Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../skills/fix-issue/findings/mdl-other.jsonl | 1 + .../refs-graph-members-enums-workflows.mdl | 117 ++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 mdl-examples/bug-tests/refs-graph-members-enums-workflows.mdl diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..5c68efc42 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area": "mdl/executor", "date": "2026-09-26", "symptom": "`refs`/`impact` printed a row per edge (a microflow with two retrieves of an entity listed twice), the impact summary counted rows (\"MICROFLOW: 9\" over six microflows) in map order that changed between runs, and `context Mod.Entity` said \"Related Entities: (none found)\" for an entity with five associations.", "cause": "No DISTINCT; summary built by ranging over a map of row counts; the context query read refs rows whose SOURCE is an entity, but an association edge's source is the ASSOCIATION, so only generalizations could ever match.", "file": "mdl/executor/cmd_search.go (showReferences, showImpact), reference_target.go (refTargetWhere, noReferencesMessage), cmd_context.go (assembleEntityContext, assembleEnumerationContext)", "insight": "An empty answer from a partial graph must say what was searched, not 'not referenced' \u2014 the wording is part of the correctness of a reference tool, because the caller acts on it by deleting. For attributes and enumeration values the message names the unresolved sites (member via a variable in an expression; decision branch on an enum, which stores the bare value name) and says to run `search`. Impact on an enumeration must include its values (TargetName LIKE 'Enum.%' with ESCAPE, since ENUM_ names contain LIKE's `_` wildcard). For related entities read the associations table, which has both ends and exists in a fast catalog. The exec* functions call ensureCatalog, which needs a connection, so the query/format half is split out (showReferences/showImpact) to be testable against a seeded catalog.New()."} diff --git a/.claude/skills/fix-issue/findings/mdl-other.jsonl b/.claude/skills/fix-issue/findings/mdl-other.jsonl index 80bffaba6..286beca7f 100644 --- a/.claude/skills/fix-issue/findings/mdl-other.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-other.jsonl @@ -73,3 +73,4 @@ {"area": "mdl/catalog", "date": "2026-09-25", "symptom": "Two import mappings with the same name, one excluded and one live (valid: mx check 0 errors), show up in CATALOG.IMPORT_MAPPINGS and CATALOG.SOURCE as two indistinguishable rows — integer Ids 2 and 3, no Excluded column, and both source rows the same `create or modify import mapping …` text with no `@excluded` prefix (mendixlabs/mxcli#1185)", "cause": "Three gaps: describeImportMapping/describeExportMapping never printed @excluded; import_mappings_data/export_mappings_data used an AUTOINCREMENT Id and recorded no Excluded; and buildSource enumerates DOCUMENTS but called the describe callback with only a qualified name, so every describer resolved by name and rendered the preferred twin for both rows. The last one was not mapping-specific — microflow/nanoflow/rule/page twins had identical source rows too, masked because the microflows table itself carries Id + Excluded", "file": "`mdl/catalog/builder_source.go` (`sourceItem.id`, `ElementId`), `mdl/catalog/builder.go` (`DescribeFunc` gains id), `mdl/catalog/tables.go` + `builder_modules.go` (mapping Id/Excluded, schema 14), `mdl/executor/excluded_docs.go` (`pickDescribed`, `describedMapping`), `mdl/executor/cmd_catalog.go` (`ExecContext.describeID`), `mdl/executor/cmd_{import,export}_mappings.go`, `mdl/visitor/visitor_import_export_mapping.go`, `mdl/backend/modelsdk/mapping_write.go` (by-name lookup prefers the live twin)", "insight": "**A collector that walks documents must not hand a NAME to a callback that resolves names** — #914's pickLive is correct for an interactive DESCRIBE and exactly wrong for a per-document sweep, because it maps both twins onto one. Pin by ID (ExecContext.describeID) and fall back to pickLive. **Printing a new annotation in DESCRIBE obliges the write side**: `@excluded` on a mapping was MDL059 until the visitor read it and documentAnnotations listed it; skipping that turns describe->exec into a refusal. **Check CatalogSchemaVersion against its own history comment**: the history recorded 13 (entity event handlers) while the constant said \"12\" — parallel bumps, the merge kept the lower — so caches built at 12 never rebuilt; bumping to 14 carries both. **Making a twin for a real run**: MDL cannot (CREATE refuses a taken name, no RENAME for mappings); `@excluded create` under a second name, then rewrite that unit's BSON Name (bson.D round-trip of the .mxunit) — mx check stays 0 errors, which is the reporter's state. Pre-fix binary on that project: two rows Id 2/3 and identical source text; fixed: document Ids, Excluded 0/1, excluded row starts `@excluded`. Controls: disable the describeID pin (4 executor tests fail, microflow included), drop the @excluded print, pass \"\" as id in buildSource (1 source row, not 2), stash tables.go/builder_modules.go (no such column: Excluded)", "refs": ["mendixlabs/mxcli#1185", "#914"]} {"area": "mdl/linter", "date": "2026-09-25", "symptom": "No way to ask a structural question about ONE document: `lint` scopes by module and by rule but not by document, so validating a microflow after each `exec` meant a project-wide lint (~13 s measured by the reporter) plus a baseline diff to see which findings were new. At that price the gate gets batched to once per session \u2014 three CONV011 violations shipped under a clean mxbuild log, six across three microflows before anyone looked.", "cause": "Feature gap, but the shape of the fix is not obvious: every rule guards its expensive per-document read (`FullMicroflow`) with `IsExcluded(moduleName)`, so module scoping already skipped the costly part for other modules \u2014 the missing granularity was WITHIN a module.", "file": "`mdl/linter/context.go` (`SetIncludedDocuments`, `IsDocumentExcluded`, `documentFilterSQL`; `Microflows`/`Pages`/`Widgets` narrowed in SQL; new `includeActive` flag), `cmd/mxcli/cmd_lint.go` + `main.go` (`-d/--documents`), `cmd/mxcli/lint_document_filter.go` (violation post-filter), tests `mdl/linter/context_document_filter_test.go`", "insight": "**Scope in the ITERATOR, not in each rule** \u2014 one SQL predicate on `Microflows()` covers CONV011, MPR002, CONV010, QUAL003 and every other rule that walks it, with no rule edited and no second copy of the filter to drift. **Two traps, both found by tests I wrote and then tried to break.** (1) An empty inclusion map means 'no filter' to `IsExcluded`, so intersecting `--modules A` with `--documents B.C` \u2014 an empty set \u2014 made lint scan the WHOLE project instead of nothing; distinguishing 'no allowlist' from 'allowlist that matched nothing' needs an explicit bool, not `len(map) > 0`. (2) The narrowing test PASSED against code with the SQL filter reverted, because the shared fixture has one microflow per module and the module implication alone explained the result \u2014 a sibling document in the SAME module is the only fixture that isolates document narrowing from module narrowing. Prove-by-revert is what caught both; the second would otherwise have shipped a test that could never fail. Also: a rule that reports from project settings rather than a document iterator is untouched by SQL narrowing, so a scoped run needs a violation post-filter too \u2014 and it must accept BOTH spellings of Location.DocumentName, since CONV010 sets the qualified name and MPR011 the short one.", "refs": ["ako/mxcli#681", "mendixlabs/mxcli#1186"]} {"area": "mdl/catalog", "date": "2026-09-25", "symptom": "java_actions.ReturnType showed a type-parameter return as the type parameter's own name: 'TypeParameter', 'TypeParEntity', 'FileTypeDocument' depending on what the modeler called it, and a type parameter named `String` (Studio Pro allows it) read back as 'String', identical to the primitive. java_action_parameters.ParameterType had the same ambiguity for both the object parameter and the entity-type selector.", "cause": "The catalog stored TypeString(), which is DESCRIBE's MDL rendering: a bare type-parameter reference IS its name in MDL syntax, so the value carried no marker that it was a type parameter at all.", "file": "mdl/catalog/builder_modules.go (catalogCodeActionType)", "insight": "The report reads like three inconsistent conventions ('TypeParameter' / 'TypeParEntity' / a name) but it is one: every value was the modeler's chosen name, and 'TypeParameter' is merely Studio Pro's default. Fix the encoding in the catalog only (`TypeParameter:`, `EntityTypeParameter:`, the `Kind:Name` shape microflows_data already uses; no primitive contains a colon) \u2014 changing TypeString() would change DESCRIBE output, where the bare name is the syntax. Test with the name colliding with a primitive and a primitive action as control: a unit test on the builder with MockBackend.ListJavaActionsFullFunc, plus an end-to-end catalog query on testdata/expr-checker (copy the whole dir; the .mpr alone is v2 without mprcontents/). MDL itself cannot declare a type parameter named String (`entity ` is a parse error), so the colliding case is only reachable via Studio Pro-authored models.", "refs": ["mendixlabs/mxcli#1183"]} +{"area": "mdl/catalog", "date": "2026-09-26", "symptom": "`impact Module.Entity.Attr` answered \"(no impact - element is not referenced)\" for an attribute a change activity sets and a page displays (Evora Factory Management: DigitalTwin.Machine.NumberOfIncidents); `impact` on an enumeration said the same; `callers` of a workflow started by a microflow said \"(no callers found)\". An agent auditing the app read these as safe-to-delete.", "cause": "The refs graph stopped at documents: no ATTRIBUTE / ENUMERATION / ENUMERATION_VALUE targets at all, and no edge for WorkflowCallAction, for a page navigating an association, or for an import/export mapping mapping an entity. Page/snippet XPath constraints also had no TargetEntity, because resolveEntityRefFromBSON read EntityRef.QualifiedName, a key no stored DirectEntityRef carries (it is `Entity`; an IndirectEntityRef ends on its last step's DestinationEntity).", "file": "mdl/catalog/builder_member_refs.go (memberRefsInUnit, scanPaths, extractXPathRefs, extractEnumerationTypeRefs), builder_references.go (microflowActionRef WorkflowCallAction), builder_xpath.go (resolveEntityRefFromBSON), catalogdb.go (CatalogTx.Query)", "insight": "Member references are found by a RAW-document walk matching every string value against the names the model declares (whole-string = structured ref: MemberChange.Attribute, AttributeRef.Attribute, EntityRefStep.Association, EnumerationType.Enumeration, ObjectMappingElement.Entity; path tokens inside expressions = association paths and qualified enum values). A typed walk would reach only the sites someone wrote a case for; the raw walk reached 4043 attribute bindings on Evora with no per-type code, and exact-set matching means prose cannot produce an edge (Documentation is skipped anyway; the test's control is an unused attribute that must stay unreferenced). XPath is resolved separately because its context entity IS known: bare names resolve against the target entity and its generalizations, predicates after a path switch context, and an enum attribute compared to a literal names the value. What stays invisible is a bare member through a variable in an expression ($Order/Total) \u2014 so the executor must not say 'not referenced' (see the executor finding). New member kinds deliberately stay OUT of graphRefKinds and off graph_god_nodes' asset side: attributes are members, not assets, and reusing change/create/retrieve for them would have pulled every attribute into communities/centrality. Bump CatalogSchemaVersion for any new edge (refs are only written by REFRESH CATALOG FULL). Verified the lint output on Evora is byte-identical in counts before/after, so no rule changed verdicts silently."} diff --git a/mdl-examples/bug-tests/refs-graph-members-enums-workflows.mdl b/mdl-examples/bug-tests/refs-graph-members-enums-workflows.mdl new file mode 100644 index 000000000..00c2c6149 --- /dev/null +++ b/mdl-examples/bug-tests/refs-graph-members-enums-workflows.mdl @@ -0,0 +1,117 @@ +-- ============================================================================ +-- refs / impact / context: gaps in the cross-reference graph +-- ============================================================================ +-- +-- Found by an agent-orientation audit on Evora Factory Management. Before: +-- +-- impact DigitalTwin.Machine.NumberOfIncidents +-- -> (no impact - element is not referenced) +-- but a change activity sets it and a page displays it +-- impact DigitalTwin.ENUM_MachineStatus -> not referenced +-- callers AltairIntegration.WF_ScheduleTechnicianAppointment +-- -> (no callers found), though a microflow calls it +-- impact DigitalTwin.Machine +-- -> ProductionLine_Reset | retrieve printed twice; "MICROFLOW: 9" +-- over six microflows; summary order changed between runs +-- context DigitalTwin.Machine -> "Related Entities: (none found)", +-- with five associations +-- +-- The graph had no ATTRIBUTE / ENUMERATION / ENUMERATION_VALUE targets, no +-- microflow -> workflow edge, no page -> association edge and no mapping -> +-- entity edge; refs/impact printed a row per edge; context read association +-- edges from the wrong end. +-- +-- ---------------------------------------------------------------------------- +-- Running this example +-- +-- mxcli exec refs-graph-members-enums-workflows.mdl -p app.mpr +-- mxcli -p app.mpr -c "refresh catalog full" +-- mxcli impact -p app.mpr RefsGraph.Machine.Incidents +-- # MICROFLOW RefsGraph.ACT_Machine_Reset (member) +-- # PAGE RefsGraph.Machine_Details (member) +-- mxcli impact -p app.mpr RefsGraph.Machine.Status +-- # MICROFLOW RefsGraph.ACT_Machine_Reset (xpath) — the retrieve's constraint +-- mxcli impact -p app.mpr RefsGraph.MachineStatus +-- # ENTITY RefsGraph.Machine (type), MICROFLOW RefsGraph.ACT_Machine_Reset +-- # (value / xpath) — uses of the values are included +-- mxcli callers -p app.mpr RefsGraph.WF_Repair +-- # RefsGraph.ACT_Machine_Repair +-- mxcli impact -p app.mpr RefsGraph.Machine +-- # ACT_Machine_Reset listed once per kind; summary counts elements +-- mxcli context -p app.mpr RefsGraph.Machine +-- # Related Entities: RefsGraph.Line and RefsGraph.Incident, with the +-- # association and its direction +-- mxcli impact -p app.mpr RefsGraph.Machine.Unused +-- # "no references found to attribute ..." plus what was and was not +-- # checked — never "not referenced" +-- ============================================================================ + +create module RefsGraph; + +create enumeration RefsGraph.MachineStatus ( + Active 'Active', + Critical 'Critical' +); + +create persistent entity RefsGraph.Line ( + Name: String(100) +); +/ + +create persistent entity RefsGraph.Machine ( + Name: String(100), + Status: Enumeration(RefsGraph.MachineStatus), + Incidents: Integer, + Unused: String(20) +); +/ + +create persistent entity RefsGraph.Incident ( + Title: String(200) +); +/ + +create association RefsGraph.Machine_Line from RefsGraph.Machine to RefsGraph.Line; +create association RefsGraph.Incident_Machine from RefsGraph.Incident to RefsGraph.Machine; + +-- Two retrieves of the same entity: one `retrieve` reference, not two rows. +create microflow RefsGraph.ACT_Machine_Reset ($Line: RefsGraph.Line) +begin + retrieve $Critical from RefsGraph.Machine where [Status = 'Critical']; + retrieve $All from RefsGraph.Machine where [RefsGraph.Machine_Line = $Line]; + loop $M in $All + begin + if $M/Status = RefsGraph.MachineStatus.Critical then + change $M (Incidents = 0); + end if; + end loop; +end; +/ + +create page RefsGraph.Machine_Details +( + params: { $Machine: RefsGraph.Machine }, + title: 'Machine', + layout: Atlas_Core.Atlas_Default +) +{ + dataview dvMachine (DataSource: $Machine) { + textbox tbName (Label: 'Name', Attribute: Name) + textbox tbIncidents (Label: 'Incidents', Attribute: Incidents) + } +} + +create workflow RefsGraph.WF_Repair + parameter $Context: RefsGraph.Machine +begin + user task Inspect 'Inspect the machine' + page RefsGraph.Machine_Details + outcomes + 'Done' { }; +end workflow; + +create microflow RefsGraph.ACT_Machine_Repair ($Machine: RefsGraph.Machine) +begin + $Wf = call workflow RefsGraph.WF_Repair ($Machine); +end; +/ From 8db1dce4363ec8a34da6c9ca167f8d4e3ca5b51d Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:26:21 +0000 Subject: [PATCH 09/13] fix(cli): keep --json stdout pure across the query commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--json` output did not parse. refs, callers, callees, impact, structure, show, search and `-c "select …"` printed "Connected to:", catalog load/build progress, a header and a "Found N" count ahead of the payload; an empty answer was a sentence ("(no references found)") instead of []; `context` ignored --json; `check --format json` wrote its document to stderr, one per phase; `diff --json` printed a text diff. The executor writes the payload and its commentary through the same ctx.Output, so the #904 mechanism (pick the executor's writer at the cmd layer) cannot separate them for commands whose payload the executor itself prints. Add ExecContext.progress(): Output in text mode (so interactive output is unchanged), the diagnostics stream (stderr) when Output carries JSON. The Diagnostics field is the one ako/mxcli#716 introduces, with the same semantics. - Status/progress in connect, catalog load/build, catalog queries and the refs/callers/callees/impact handlers goes through progress(). - Empty results go through writeEmptyResult: [] in JSON mode. - context --json wraps the markdown in {name, type, depth, context}. - show catalog status / show widgets gain a JSON path; empty listings of data transformers, import/export mappings and navigation follow the existing `&& ctx.Format != FormatJSON` idiom. - search: --json is the canonical spelling; --format json is kept as a deprecated alias and now sets the executor format, so it is as clean. - A cold-cache catalog build under -q moves its per-table lines to stderr instead of stdout (search -q --format names). - check: structured formats emit one document on stdout covering every phase; the exit code is unchanged. - diff / diff-local refuse --json (no JSON output) instead of ignoring it. Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/cmd_check.go | 44 ++- cmd/mxcli/cmd_diff.go | 2 + cmd/mxcli/cmd_query.go | 18 +- cmd/mxcli/json_output_purity_test.go | 329 ++++++++++++++++++++++ cmd/mxcli/main.go | 2 +- cmd/mxcli/output_stream.go | 15 + docs-site/src/internals/fts5.md | 2 +- docs-site/src/reference/query/search.md | 2 +- docs-site/src/tools/search.md | 18 +- docs-site/src/tutorial/describe-search.md | 5 +- mdl/executor/cmd_catalog.go | 88 ++++-- mdl/executor/cmd_context.go | 17 ++ mdl/executor/cmd_datatransformer.go | 2 +- mdl/executor/cmd_export_mappings.go | 2 +- mdl/executor/cmd_import_mappings.go | 2 +- mdl/executor/cmd_navigation.go | 2 +- mdl/executor/cmd_search.go | 42 ++- mdl/executor/cmd_widgets.go | 6 +- mdl/executor/exec_context.go | 31 ++ mdl/executor/executor_connect.go | 2 +- mdl/executor/format.go | 14 + mdl/executor/progress_stream_test.go | 56 ++++ mdl/executor/reference_target.go | 2 +- 23 files changed, 630 insertions(+), 73 deletions(-) create mode 100644 cmd/mxcli/json_output_purity_test.go create mode 100644 mdl/executor/progress_stream_test.go diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index 59cea5afc..3b74568a2 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -79,9 +79,10 @@ Examples: # Scan the project for legacy native widgets after a Mendix upgrade mxcli check script.mdl -p app.mpr --post-migration - # Output as JSON or SARIF + # Output as JSON or SARIF: one document on stdout covering every phase + # (progress goes to stderr); the exit code still says whether it passed mxcli check script.mdl --format json - mxcli check script.mdl --format sarif + mxcli check script.mdl -p app.mpr --format sarif > results.sarif # Read the script from stdin cat script.mdl | mxcli check - @@ -105,6 +106,21 @@ Examples: outputFormat := linter.OutputFormat(format) formatter := linter.GetFormatter(outputFormat, !isStructured) + // In a structured format the payload is ONE document on stdout, emitted + // once at the end (or at the first failing phase). Each phase used to + // format its own violations to stderr, so `check --format json` put + // nothing parseable on stdout — only the executor's "Connected to:" + // chatter — and a run reaching several phases wrote several documents. + var structured []linter.Violation + finish := func(code int) { + if isStructured { + formatter.Format(structured, os.Stdout) + } + if code != 0 { + os.Exit(code) + } + } + // Read the script (a path, or "-" for stdin) content, err := readMDLSource(filePath) if err != nil { @@ -160,7 +176,8 @@ Examples: Message: parseErr.Error(), }) } - formatter.Format(parseViolations, os.Stderr) + structured = append(structured, parseViolations...) + finish(1) } else { fmt.Fprintf(os.Stderr, "Syntax errors found:\n") for _, err := range errs { @@ -197,7 +214,7 @@ Examples: if isStructured { // Always emit structured output (even when clean) - formatter.Format(violations, os.Stderr) + structured = append(structured, violations...) } else if len(violations) > 0 { fmt.Fprintln(os.Stderr) formatter.Format(violations, os.Stderr) @@ -206,7 +223,7 @@ Examples: if len(violations) > 0 { summary := linter.Summarize(violations) if summary.Errors > 0 { - os.Exit(1) + finish(1) } } @@ -221,7 +238,7 @@ Examples: fmt.Printf("\nValidating references against: %s\n", projectPath) fmt.Printf("(Note: References to objects created within the script are skipped)\n") } - exec, logger := newLoggedExecutor("check") + exec, logger := newLoggedExecutorTo("check", progressSink(format)) defer logger.Close() defer exec.Close() @@ -258,7 +275,7 @@ Examples: fmt.Fprintf(os.Stderr, " %s\n", w) } } else if len(warnViolations) > 0 && len(validationErrors) == 0 { - formatter.Format(warnViolations, os.Stderr) + structured = append(structured, warnViolations...) } if len(validationErrors) > 0 { @@ -271,7 +288,7 @@ Examples: Message: err.Error(), }) } - formatter.Format(refViolations, os.Stderr) + structured = append(structured, refViolations...) } else { fmt.Fprintf(os.Stderr, "Reference errors:\n") for _, err := range validationErrors { @@ -279,7 +296,7 @@ Examples: } fmt.Fprintf(os.Stderr, "\n✗ %d reference error(s) found\n", len(validationErrors)) } - os.Exit(1) + finish(1) } if !isStructured { fmt.Printf("✓ All references valid\n") @@ -310,13 +327,13 @@ Examples: projectViolations = append(projectViolations, exec.TypeCheckProgram(prog)...) if len(projectViolations) > 0 { if isStructured { - formatter.Format(projectViolations, os.Stderr) + structured = append(structured, projectViolations...) } else { fmt.Fprintln(os.Stderr) formatter.Format(projectViolations, os.Stderr) } if linter.Summarize(projectViolations).Errors > 0 { - os.Exit(1) + finish(1) } } else if !isStructured { fmt.Printf("✓ Expression types OK, no unstated member drops\n") @@ -340,7 +357,7 @@ Examples: os.Exit(1) } if isStructured { - formatter.Format(legacyViolations, os.Stderr) + structured = append(structured, legacyViolations...) } else if len(legacyViolations) > 0 { fmt.Fprintln(os.Stderr) formatter.Format(legacyViolations, os.Stderr) @@ -351,11 +368,12 @@ Examples: if len(legacyViolations) > 0 { summary := linter.Summarize(legacyViolations) if summary.Errors > 0 { - os.Exit(1) + finish(1) } } } + finish(0) if !isStructured { fmt.Println("\nCheck passed!") // Qualify the verdict when nothing was resolved against a model. A diff --git a/cmd/mxcli/cmd_diff.go b/cmd/mxcli/cmd_diff.go index c48712b7d..fc5402aa1 100644 --- a/cmd/mxcli/cmd_diff.go +++ b/cmd/mxcli/cmd_diff.go @@ -44,6 +44,7 @@ Examples: format, _ := cmd.Flags().GetString("format") useColor, _ := cmd.Flags().GetBool("color") width, _ := cmd.Flags().GetInt("width") + refuseJSONFlag("diff", "--format unified|side|struct") if projectPath == "" { fmt.Fprintln(os.Stderr, "Error: --project (-p) is required") @@ -129,6 +130,7 @@ Examples: format, _ := cmd.Flags().GetString("format") useColor, _ := cmd.Flags().GetBool("color") width, _ := cmd.Flags().GetInt("width") + refuseJSONFlag("diff-local", "--format unified|side|struct") if projectPath == "" { fmt.Fprintln(os.Stderr, "Error: --project (-p) is required") diff --git a/cmd/mxcli/cmd_query.go b/cmd/mxcli/cmd_query.go index d99b7fbf0..61dcd025e 100644 --- a/cmd/mxcli/cmd_query.go +++ b/cmd/mxcli/cmd_query.go @@ -7,6 +7,7 @@ import ( "os" "strings" + "github.com/mendixlabs/mxcli/mdl/executor" "github.com/mendixlabs/mxcli/mdl/visitor" "github.com/spf13/cobra" ) @@ -181,11 +182,16 @@ Supported element types: Use --depth to control how deep to traverse call chains (default: 2). +With --json the assembled context is wrapped in one JSON object, +{"name", "type", "depth", "context"}, where "context" is the markdown text +above; progress goes to stderr. + Examples: mxcli context -p app.mpr Module.ProcessOrder mxcli context -p app.mpr Module.Customer --depth 3 mxcli context -p app.mpr Module.OrderPage mxcli context -p app.mpr Module.ImportCsvData + mxcli context -p app.mpr Module.Customer --json | jq -r .context `, Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { @@ -216,10 +222,13 @@ definitions. Requires at least a FULL catalog build (done automatically). Output Formats: table - Human-readable table (default) names - Just qualified names, one per line (for piping) - json - JSON output + --json - JSON array on stdout, progress on stderr (the same flag every + query command takes). "--format json" is still accepted as a + deprecated spelling of it. Examples: mxcli search -p app.mpr "validation" + mxcli search -p app.mpr "Customer" --json | jq '.[].qualifiedName' mxcli search -p app.mpr "Customer" --format names mxcli search -p app.mpr "error" -q --format names | xargs -I {} mxcli describe -p app.mpr microflow {} `, @@ -234,8 +243,15 @@ Examples: os.Exit(1) } + // A JSON-format executor sends "Connected to:" and catalog progress to + // stderr (ExecContext.progress). Set from the resolved format, not + // from --json alone: `--format json` is the older spelling of the same + // request and must be exactly as clean. exec, logger := newLoggedExecutor("subcommand") defer logger.Close() + if isMachineReadableFormat(format) { + exec.SetFormat(executor.FormatJSON) + } if quiet { exec.SetQuiet(true) } diff --git a/cmd/mxcli/json_output_purity_test.go b/cmd/mxcli/json_output_purity_test.go new file mode 100644 index 000000000..efe58f4a1 --- /dev/null +++ b/cmd/mxcli/json_output_purity_test.go @@ -0,0 +1,329 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "encoding/json" + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// `--json` is a contract with a program, not a style: whatever reaches stdout is +// handed to a JSON parser. It was broken on the query family — refs, callers, +// select and friends printed "Connected to: …", catalog progress and a header +// ahead of the payload, `context` ignored the flag entirely, and an empty answer +// came back as the words "(no references found)". Each command looked fine to a +// person, which is why nothing caught it. +// +// These run the real main() in a child process (see TestRunMainHelper), so the +// two streams are genuinely separate and an error path that ends in os.Exit is +// observable. The table is the guard: a new subcommand that prints its payload +// through the executor belongs in it. + +// jsonPurityFixture copies the small v1 test project into a fresh directory, so +// the first command against it builds the catalog cold — the path that prints +// the most progress — and nothing is written next to the checked-in fixture. +func jsonPurityFixture(t *testing.T) string { + t.Helper() + src := filepath.Join("..", "..", "modelsdk", "mpr", "testdata", "v1-project", "App.mpr") + data, err := os.ReadFile(src) + if err != nil { + t.Skipf("fixture project not available: %v", err) + } + dir := t.TempDir() + mpr := filepath.Join(dir, "App.mpr") + if err := os.WriteFile(mpr, data, 0o644); err != nil { + t.Fatal(err) + } + return mpr +} + +type mainResult struct { + stdout, stderr string + exitCode int +} + +// runMainStreams runs mxcli in a child process and returns both streams and the +// exit code. +func runMainStreams(t *testing.T, dir string, args ...string) mainResult { + t.Helper() + enc, _ := json.Marshal(args) + cmd := exec.Command(os.Args[0], "-test.run=^TestRunMainHelper$") + cmd.Dir = dir + cmd.Env = append(os.Environ(), + runMainEnv+"="+string(enc), + "MXCLI_LOG_DIR="+t.TempDir(), + "MXCLI_QUIET=1", + ) + cmd.Stdin = strings.NewReader("") + var out, errb bytes.Buffer + cmd.Stdout, cmd.Stderr = &out, &errb + err := cmd.Run() + code := 0 + var ee *exec.ExitError + if errors.As(err, &ee) { + code = ee.ExitCode() + } else if err != nil { + t.Fatalf("run mxcli %v: %v", args, err) + } + return mainResult{stdout: out.String(), stderr: errb.String(), exitCode: code} +} + +func TestJSONFlagKeepsStdoutPureJSON(t *testing.T) { + if testing.Short() { + t.Skip("spawns mxcli against a project") + } + mpr := jsonPurityFixture(t) + dir := filepath.Dir(mpr) + + cases := []struct { + name string + args []string + // empty: the answer is "nothing", which must still be JSON ([]), not a sentence. + empty bool + }{ + // First in the list on purpose: the catalog is cold, so this run builds + // it and prints every "✓ Table: N" progress line. + {name: "refs (cold catalog)", args: []string{"refs", "System.User"}}, + {name: "refs", args: []string{"refs", "System.User"}}, + {name: "refs, nothing found", args: []string{"refs", "Nope.Nothing"}, empty: true}, + {name: "callers", args: []string{"callers", "System.User"}, empty: true}, + {name: "callers transitive", args: []string{"callers", "System.User", "--transitive"}, empty: true}, + {name: "callees", args: []string{"callees", "System.User"}, empty: true}, + {name: "impact", args: []string{"impact", "System.User"}}, + {name: "impact, nothing found", args: []string{"impact", "Nope.Nothing"}, empty: true}, + {name: "context", args: []string{"context", "System.User"}}, + {name: "structure", args: []string{"structure"}}, + {name: "show entities", args: []string{"show", "entities"}}, + {name: "show modules", args: []string{"show", "modules"}}, + {name: "describe", args: []string{"describe", "entity", "System.User"}}, + {name: "search", args: []string{"search", "User"}}, + {name: "search, nothing found", args: []string{"search", "zzqqxxnomatch"}, empty: true}, + {name: "select", args: []string{"-c", "select Name from CATALOG.entities order by Name limit 3"}}, + {name: "select, nothing found", args: []string{"-c", "select Name from CATALOG.entities where Name = 'zz'"}, empty: true}, + {name: "select, source-only table", args: []string{"-c", "select QualifiedName from CATALOG.source limit 1"}, empty: true}, + {name: "-c show references", args: []string{"-c", "show references to System.User"}}, + {name: "show catalog tables", args: []string{"-c", "show catalog tables"}}, + {name: "show catalog status", args: []string{"-c", "show catalog status"}}, + // Empty listings: each of these answered "No … found." under --json. + {name: "show widgets, none", args: []string{"-c", "show widgets"}, empty: true}, + {name: "show data transformers, none", args: []string{"-c", "show data transformers"}, empty: true}, + {name: "show import mappings, none", args: []string{"-c", "show import mappings"}, empty: true}, + {name: "show export mappings, none", args: []string{"-c", "show export mappings"}, empty: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + args := append([]string{"-p", mpr, "--json"}, tc.args...) + r := runMainStreams(t, dir, args...) + if r.exitCode != 0 { + t.Fatalf("mxcli %v exited %d\nstderr:\n%s", args, r.exitCode, r.stderr) + } + assertPureJSON(t, args, r) + if tc.empty && strings.TrimSpace(r.stdout) != "[]" { + t.Errorf("mxcli %v: an empty answer must be [] on stdout, got:\n%s", args, r.stdout) + } + }) + } +} + +// search has spelled JSON two ways. `--json` is canonical (the one the whole +// query family takes); `--format json` must keep working for existing callers +// and be exactly as clean. +func TestSearchFormatJSONAliasIsAsPureAsJSONFlag(t *testing.T) { + if testing.Short() { + t.Skip("spawns mxcli against a project") + } + mpr := jsonPurityFixture(t) + dir := filepath.Dir(mpr) + + canonical := runMainStreams(t, dir, "search", "-p", mpr, "User", "--json") + alias := runMainStreams(t, dir, "search", "-p", mpr, "User", "--format", "json") + for _, r := range []mainResult{canonical, alias} { + if r.exitCode != 0 { + t.Fatalf("search exited %d\nstderr:\n%s", r.exitCode, r.stderr) + } + assertPureJSON(t, []string{"search", "User"}, r) + } + if canonical.stdout != alias.stdout { + t.Errorf("--json and --format json disagree:\n--json:\n%s\n--format json:\n%s", canonical.stdout, alias.stdout) + } +} + +// An error under --json is a non-zero exit with the message on stderr. A text +// line on stdout would be read as a (corrupt) payload. +func TestJSONFlagErrorsGoToStderr(t *testing.T) { + if testing.Short() { + t.Skip("spawns mxcli against a project") + } + mpr := jsonPurityFixture(t) + dir := filepath.Dir(mpr) + script := filepath.Join(dir, "s.mdl") + if err := os.WriteFile(script, []byte("show entities;\n"), 0o644); err != nil { + t.Fatal(err) + } + + for _, args := range [][]string{ + {"context", "Nope.Nothing"}, + {"describe", "entity", "Nope.Nothing"}, + {"-c", "select Nope from CATALOG.no_such_table"}, + // --json is a root flag, so diff accepted it and printed a text diff. + // A command with no JSON output must refuse, not ignore it. + {"diff", script}, + } { + full := append([]string{"-p", mpr, "--json"}, args...) + r := runMainStreams(t, dir, full...) + if r.exitCode == 0 { + t.Errorf("mxcli %v: expected a non-zero exit, got 0\nstdout:\n%s", full, r.stdout) + } + if s := strings.TrimSpace(r.stdout); s != "" && !json.Valid([]byte(s)) { + t.Errorf("mxcli %v: error path wrote non-JSON to stdout:\n%s", full, r.stdout) + } + if strings.TrimSpace(r.stderr) == "" { + t.Errorf("mxcli %v: error path said nothing on stderr", full) + } + } +} + +// Control: text mode is for a person and must be exactly as before — the +// progress and headers still on stdout. Without this, a fix that simply sent +// everything to stderr (or dropped it) would pass the tests above and silently +// degrade interactive use. +func TestTextModeKeepsProgressOnStdout(t *testing.T) { + if testing.Short() { + t.Skip("spawns mxcli against a project") + } + mpr := jsonPurityFixture(t) + dir := filepath.Dir(mpr) + + cases := []struct { + args []string + want []string // lines a person has always seen on stdout + }{ + {[]string{"refs", "System.User"}, []string{"Connected to:", "Building catalog", "References to System.User"}}, + {[]string{"refs", "Nope.Nothing"}, []string{"Connected to:", "(no references found)"}}, + {[]string{"callers", "System.User"}, []string{"Connected to:", "Callers of System.User", "(no callers found)"}}, + {[]string{"impact", "System.User"}, []string{"Impact analysis for System.User", "Summary:"}}, + {[]string{"context", "System.User"}, []string{"Connected to:", "## Context: System.User"}}, + {[]string{"show", "entities"}, []string{"Connected to:", "| Entity"}}, + {[]string{"search", "User"}, []string{"Loading cached catalog", "String Matches"}}, + } + for _, tc := range cases { + args := append([]string{"-p", mpr}, tc.args...) + r := runMainStreams(t, dir, args...) + if r.exitCode != 0 { + t.Fatalf("mxcli %v exited %d\nstderr:\n%s", args, r.exitCode, r.stderr) + } + for _, w := range tc.want { + if !strings.Contains(r.stdout, w) { + t.Errorf("mxcli %v: text mode lost %q from stdout\nstdout:\n%s\nstderr:\n%s", args, w, r.stdout, r.stderr) + } + } + if json.Valid([]byte(strings.TrimSpace(r.stdout))) { + t.Errorf("mxcli %v: text mode emitted JSON:\n%s", args, r.stdout) + } + } +} + +// assertPureJSON fails unless stdout is one JSON document and the progress that +// used to precede it went to stderr rather than vanishing. +func assertPureJSON(t *testing.T, args []string, r mainResult) { + t.Helper() + out := strings.TrimSpace(r.stdout) + if out == "" { + t.Fatalf("mxcli %v: --json printed nothing on stdout\nstderr:\n%s", args, r.stderr) + } + if !json.Valid([]byte(out)) { + t.Fatalf("mxcli %v: stdout is not JSON:\n%s", args, firstLines(r.stdout, 8)) + } + for _, leak := range []string{"Connected to:", "Loading cached catalog", "Building catalog", "✓ "} { + if strings.Contains(r.stdout, leak) { + t.Errorf("mxcli %v: progress %q on stdout", args, leak) + } + } +} + +// `check` is the command an agent reaches for before `exec`, and its structured +// formats are the ones it parses. They were written to stderr — one document +// per phase — so stdout held only "Connected to:" and a multi-phase run +// produced several documents. The payload is now one document on stdout, and a +// failing check still exits non-zero with its violations in that document. +func TestCheckStructuredOutputIsOneDocumentOnStdout(t *testing.T) { + if testing.Short() { + t.Skip("spawns mxcli against a project") + } + mpr := jsonPurityFixture(t) + dir := filepath.Dir(mpr) + good := filepath.Join(dir, "good.mdl") + bad := filepath.Join(dir, "bad.mdl") + if err := os.WriteFile(good, []byte("show entities;\n"), 0o644); err != nil { + t.Fatal(err) + } + // Parses, but names an entity the project does not have: fails in the + // reference phase, after the syntax phase has already contributed. + if err := os.WriteFile(bad, []byte("alter entity Nope.Thing add attribute X: String;\n"), 0o644); err != nil { + t.Fatal(err) + } + + cases := []struct { + args []string + wantExit int + }{ + {[]string{"check", good, "--json"}, 0}, + {[]string{"check", good, "--format", "json"}, 0}, + {[]string{"check", good, "-p", mpr, "--json"}, 0}, + {[]string{"check", bad, "-p", mpr, "--json"}, 1}, + {[]string{"check", good, "-p", mpr, "--format", "sarif"}, 0}, + } + for _, tc := range cases { + r := runMainStreams(t, dir, tc.args...) + if r.exitCode != tc.wantExit { + t.Errorf("mxcli %v: exit %d, want %d\nstdout:\n%s\nstderr:\n%s", tc.args, r.exitCode, tc.wantExit, r.stdout, r.stderr) + continue + } + dec := json.NewDecoder(strings.NewReader(r.stdout)) + var doc map[string]any + if err := dec.Decode(&doc); err != nil { + t.Errorf("mxcli %v: stdout is not JSON: %v\nstdout:\n%s\nstderr:\n%s", tc.args, err, r.stdout, r.stderr) + continue + } + if dec.More() { + t.Errorf("mxcli %v: more than one document on stdout:\n%s", tc.args, r.stdout) + } + if strings.Contains(r.stdout, "Connected to:") { + t.Errorf("mxcli %v: progress on stdout:\n%s", tc.args, r.stdout) + } + } +} + +// `search -q --format names` is the documented way to pipe names into another +// command. On a cold catalog the build's "✓ Table: N" lines were not gated by +// -q and landed on stdout ahead of the names. +func TestSearchQuietNamesIsOnlyNamesOnAColdCatalog(t *testing.T) { + if testing.Short() { + t.Skip("spawns mxcli against a project") + } + mpr := jsonPurityFixture(t) + r := runMainStreams(t, filepath.Dir(mpr), "search", "-p", mpr, "User", "-q", "--format", "names") + if r.exitCode != 0 { + t.Fatalf("exit %d\nstderr:\n%s", r.exitCode, r.stderr) + } + lines := strings.Split(strings.TrimRight(r.stdout, "\n"), "\n") + if len(lines) == 0 || lines[0] == "" { + t.Fatalf("no names on stdout\nstderr:\n%s", r.stderr) + } + for _, l := range lines { + if parts := strings.Split(l, "\t"); len(parts) != 2 { + t.Errorf("not a typename line: %q", l) + } + } + // The progress is moved, not dropped: a cold build can take minutes. + if !strings.Contains(r.stderr, "✓ Entities:") { + t.Errorf("build progress vanished instead of moving to stderr:\n%s", r.stderr) + } +} diff --git a/cmd/mxcli/main.go b/cmd/mxcli/main.go index ffc9fb009..e56108eed 100644 --- a/cmd/mxcli/main.go +++ b/cmd/mxcli/main.go @@ -351,7 +351,7 @@ func init() { describeCmd.Flags().StringP("format", "f", "mdl", "Output format: mdl, json, mermaid, elk") // Search command flags - searchCmd.Flags().StringP("format", "f", "table", "Output format: table, names, json") + searchCmd.Flags().StringP("format", "f", "table", "Output format: table, names (for JSON use --json; \"json\" is accepted as a deprecated alias)") searchCmd.Flags().BoolP("quiet", "q", false, "Suppress connection and status messages (for piping)") // Callers/callees command flags diff --git a/cmd/mxcli/output_stream.go b/cmd/mxcli/output_stream.go index 9693d1a84..d001d13b4 100644 --- a/cmd/mxcli/output_stream.go +++ b/cmd/mxcli/output_stream.go @@ -3,6 +3,7 @@ package main import ( + "fmt" "io" "os" "strings" @@ -45,3 +46,17 @@ func progressSink(format string) io.Writer { } return os.Stdout } + +// refuseJSONFlag exits with an error when the global --json flag was given to a +// command that has no JSON output. --json is a root persistent flag, so every +// subcommand accepts it; one that ignores it prints text to a caller that is +// about to hand stdout to a JSON parser, which is worse than saying no. Fails +// the way every --json error must: message on stderr, non-zero exit, nothing on +// stdout. +func refuseJSONFlag(command, alternatives string) { + if !globalJSONFlag { + return + } + fmt.Fprintf(os.Stderr, "Error: %s has no JSON output (--json is not supported); use %s\n", command, alternatives) + os.Exit(2) +} diff --git a/docs-site/src/internals/fts5.md b/docs-site/src/internals/fts5.md index 51e9a6845..39e6aaa10 100644 --- a/docs-site/src/internals/fts5.md +++ b/docs-site/src/internals/fts5.md @@ -90,7 +90,7 @@ mxcli search -p app.mpr "validation" mxcli search -p app.mpr "error" -q --format names # JSON output -mxcli search -p app.mpr "Customer" -q --format json +mxcli search -p app.mpr "Customer" --json ``` The search results include: diff --git a/docs-site/src/reference/query/search.md b/docs-site/src/reference/query/search.md index 1af1ab9da..52422bd8d 100644 --- a/docs-site/src/reference/query/search.md +++ b/docs-site/src/reference/query/search.md @@ -34,7 +34,7 @@ Search using the CLI: ```sql -- From the command line: -- mxcli search -p app.mpr "Customer" --format names --- mxcli search -p app.mpr "Customer" --format json +-- mxcli search -p app.mpr "Customer" --json ``` ## See Also diff --git a/docs-site/src/tools/search.md b/docs-site/src/tools/search.md index d8e40af14..3bafed053 100644 --- a/docs-site/src/tools/search.md +++ b/docs-site/src/tools/search.md @@ -37,15 +37,23 @@ mxcli search -p app.mpr "validation" -q mxcli search -p app.mpr "validation" -q --format names # Output as JSON array -mxcli search -p app.mpr "validation" -q --format json +mxcli search -p app.mpr "validation" --json ``` ## Output Formats -| Format | Description | -|--------|-------------| -| `names` (default) | `typename` per line, suitable for piping | -| `json` | JSON array of search results | +| Flag | Description | +|------|-------------| +| `--format table` (default) | Human-readable tables of string and source matches | +| `--format names` | `typename` per line, suitable for piping | +| `--json` | JSON array of search results | + +`--json` is the same flag every query command takes (`refs`, `callers`, +`callees`, `impact`, `context`, `show`, `describe`, `-c "select …"`). With it, +stdout carries the JSON document and nothing else — connection and catalog +progress go to stderr, an empty result is `[]`, and an error is a message on +stderr with a non-zero exit. `--format json` is still accepted as a deprecated +spelling of `--json`. The `-q` (quiet) flag suppresses headers and formatting, making output suitable for piping to other commands. diff --git a/docs-site/src/tutorial/describe-search.md b/docs-site/src/tutorial/describe-search.md index edf045649..8856cdf73 100644 --- a/docs-site/src/tutorial/describe-search.md +++ b/docs-site/src/tutorial/describe-search.md @@ -201,8 +201,9 @@ mxcli search -p app.mpr "validation" # Show only element names (no context) mxcli search -p app.mpr "validation" --format names -# JSON output for programmatic use -mxcli search -p app.mpr "validation" --format json +# JSON output for programmatic use (stdout is only the JSON array; +# progress goes to stderr, and no matches is []) +mxcli search -p app.mpr "validation" --json ``` The `--format names` option is useful for piping into other commands: diff --git a/mdl/executor/cmd_catalog.go b/mdl/executor/cmd_catalog.go index 6d9be1256..dff2f1186 100644 --- a/mdl/executor/cmd_catalog.go +++ b/mdl/executor/cmd_catalog.go @@ -45,7 +45,7 @@ func execShowCatalogTables(ctx *ExecContext) error { } tables := ctx.Catalog.Tables() - fmt.Fprintf(ctx.Output, "\nFound %d catalog table(s)\n", len(tables)) + fmt.Fprintf(ctx.progress(), "\nFound %d catalog table(s)\n", len(tables)) // Get row counts for each table type tableInfo struct { @@ -157,13 +157,13 @@ func warnIfCatalogModeInsufficient(ctx *ExecContext, query string) { currentRank := modeRank[buildMode] if communitiesOnlyTables[table] && !graphAnalysis { - fmt.Fprintf(ctx.Output, "Warning: CATALOG.%s requires refresh catalog communities (not run for this catalog)\n", strings.ToUpper(table)) + fmt.Fprintf(ctx.progress(), "Warning: CATALOG.%s requires refresh catalog communities (not run for this catalog)\n", strings.ToUpper(table)) return } if sourceOnlyTables[table] && currentRank < modeRank["source"] { - fmt.Fprintf(ctx.Output, "Warning: CATALOG.%s requires refresh catalog full source (current mode: %s)\n", strings.ToUpper(table), buildMode) + fmt.Fprintf(ctx.progress(), "Warning: CATALOG.%s requires refresh catalog full source (current mode: %s)\n", strings.ToUpper(table), buildMode) } else if fullOnlyTables[table] && currentRank < modeRank["full"] { - fmt.Fprintf(ctx.Output, "Warning: CATALOG.%s requires refresh catalog full (current mode: %s)\n", strings.ToUpper(table), buildMode) + fmt.Fprintf(ctx.progress(), "Warning: CATALOG.%s requires refresh catalog full (current mode: %s)\n", strings.ToUpper(table), buildMode) } } @@ -189,10 +189,9 @@ func execCatalogQuery(ctx *ExecContext, query string) error { } // Output results - fmt.Fprintf(ctx.Output, "Found %d result(s)\n", result.Count) + fmt.Fprintf(ctx.progress(), "Found %d result(s)\n", result.Count) if result.Count == 0 { - fmt.Fprintln(ctx.Output, "(no results)") - return nil + return writeEmptyResult(ctx, result.Columns, "(no results)") } outputCatalogResults(ctx, result) @@ -425,9 +424,9 @@ func loadCachedCatalog(ctx *ExecContext, cachePath string) error { ctx.Catalog = cat if !ctx.Quiet { age := time.Since(info.BuildTime) - fmt.Fprintf(ctx.Output, "Loading cached catalog (built %s ago, %s mode)...\n", + fmt.Fprintf(ctx.progress(), "Loading cached catalog (built %s ago, %s mode)...\n", formatDuration(age), info.BuildMode) - fmt.Fprintf(ctx.Output, "✓ Catalog ready (from cache)\n") + fmt.Fprintf(ctx.progress(), "✓ Catalog ready (from cache)\n") } return nil } @@ -476,17 +475,17 @@ func buildCatalog(ctx *ExecContext, full, isSource, communities bool, resolution if carry, res := carryGraphAnalysis(cachedCatalogInfo(ctx), full, communities); carry { communities, resolution = true, res if !ctx.Quiet { - fmt.Fprintln(ctx.Output, "Carrying over graph analysis (this catalog has it)...") + fmt.Fprintln(ctx.progress(), "Carrying over graph analysis (this catalog has it)...") } } if !ctx.Quiet { if isSource { - fmt.Fprintln(ctx.Output, "Building catalog (source mode - includes MDL source)...") + fmt.Fprintln(ctx.progress(), "Building catalog (source mode - includes MDL source)...") } else if full { - fmt.Fprintln(ctx.Output, "Building catalog (full mode - includes activities/widgets)...") + fmt.Fprintln(ctx.progress(), "Building catalog (full mode - includes activities/widgets)...") } else { - fmt.Fprintln(ctx.Output, "Building catalog (fast mode)...") + fmt.Fprintln(ctx.progress(), "Building catalog (fast mode)...") } } start := time.Now() @@ -519,8 +518,17 @@ func buildCatalog(ctx *ExecContext, full, isSource, communities bool, resolution // Project-level widgets (widgets/*.mpk) are added by the catalog builder // itself by reading the filesystem. builder.SetBuiltinWidgetMetas(builtinWidgetMetas()) + // Per-table lines are the only sign of life during a build that takes + // minutes on a large app (18 on a 140MB one), so quiet mode moves them off + // stdout rather than dropping them: `search -q --format names` promises one + // name per line, and forty "✓ Table: N" lines ahead of the names on a cold + // cache broke that promise. + buildProgress := ctx.progress() + if ctx.Quiet { + buildProgress = ctx.diagnostics() + } err = builder.Build(func(table string, count int) { - fmt.Fprintf(ctx.Output, "✓ %s: %d\n", table, count) + fmt.Fprintf(buildProgress, "✓ %s: %d\n", table, count) }) if err != nil { cat.Close() @@ -542,7 +550,7 @@ func buildCatalog(ctx *ExecContext, full, isSource, communities bool, resolution ctx.Catalog = cat if !ctx.Quiet { - fmt.Fprintf(ctx.Output, "✓ Catalog ready (%.1fs)\n", elapsed.Seconds()) + fmt.Fprintf(ctx.progress(), "✓ Catalog ready (%.1fs)\n", elapsed.Seconds()) } // Save to cache file — unless doing so would narrow it. @@ -563,7 +571,7 @@ func buildCatalog(ctx *ExecContext, full, isSource, communities bool, resolution if cachePath != "" { if existing := cachedCatalogMode(ctx); catalogModeRank(existing) > catalogModeRank(buildMode) { if !ctx.Quiet { - fmt.Fprintf(ctx.Output, "Keeping the existing %s-mode catalog cache (this build was %s mode)\n", existing, buildMode) + fmt.Fprintf(ctx.progress(), "Keeping the existing %s-mode catalog cache (this build was %s mode)\n", existing, buildMode) } return nil } @@ -572,9 +580,9 @@ func buildCatalog(ctx *ExecContext, full, isSource, communities bool, resolution // Remove existing cache file first os.Remove(cachePath) if err := cat.SaveToFile(cachePath); err != nil { - fmt.Fprintf(ctx.Output, "Warning: failed to save catalog cache: %v\n", err) + fmt.Fprintf(ctx.progress(), "Warning: failed to save catalog cache: %v\n", err) } else { - fmt.Fprintf(ctx.Output, "✓ Catalog cached to %s\n", cachePath) + fmt.Fprintf(buildProgress, "✓ Catalog cached to %s\n", cachePath) } } } @@ -738,6 +746,9 @@ func execRefreshCatalog(ctx *ExecContext, full bool) error { // execShowCatalogStatus handles SHOW CATALOG STATUS command. func execShowCatalogStatus(ctx *ExecContext) error { + if ctx.Format == FormatJSON { + return writeCatalogStatusJSON(ctx) + } cachePath := getCachePath(ctx) if cachePath == "" { fmt.Fprintln(ctx.Output, "No project connected") @@ -806,6 +817,47 @@ func execShowCatalogStatus(ctx *ExecContext) error { return nil } +// writeCatalogStatusJSON is SHOW CATALOG STATUS for --json: the same facts as +// the text report, as one object. The report is prose ("Status: ✓ Valid"), so +// under --json it used to be the only thing on stdout and did not parse. +func writeCatalogStatusJSON(ctx *ExecContext) error { + cachePath := getCachePath(ctx) + if cachePath == "" { + return mdlerrors.NewNotConnected() + } + status := map[string]any{"cachePath": cachePath, "exists": false} + if _, err := os.Stat(cachePath); err == nil { + status["exists"] = true + cat, err := catalog.NewFromFile(cachePath) + if err != nil { + return mdlerrors.NewBackend("open catalog cache", err) + } + defer cat.Close() + info, err := cat.GetCacheInfo() + if err != nil { + return mdlerrors.NewBackend("read catalog cache info", err) + } + valid, reason := isCacheValid(ctx, cachePath, "fast") + status["buildMode"] = info.BuildMode + status["buildTime"] = info.BuildTime + status["buildDuration"] = info.BuildDuration.String() + status["mendixVersion"] = info.MendixVersion + status["valid"] = valid + if !valid { + status["invalidReason"] = reason + } + status["fullMode"] = info.BuildMode == "full" || info.BuildMode == "source" + status["sourceMode"] = info.BuildMode == "source" + status["graphAnalysis"] = info.GraphAnalysis + if info.GraphAnalysis { + status["graphResolution"] = info.GraphResolution + } + } + enc := json.NewEncoder(ctx.Output) + enc.SetIndent("", " ") + return enc.Encode(status) +} + // catalogTableRefRe matches a `CATALOG.` table reference (case-insensitive). var catalogTableRefRe = regexp.MustCompile(`(?i)\bcatalog\.([a-z_][a-z0-9_]*)`) diff --git a/mdl/executor/cmd_context.go b/mdl/executor/cmd_context.go index a0ee96522..bb533424e 100644 --- a/mdl/executor/cmd_context.go +++ b/mdl/executor/cmd_context.go @@ -3,6 +3,7 @@ package executor import ( + "encoding/json" "fmt" "strings" @@ -61,6 +62,22 @@ func execShowContext(ctx *ExecContext, s *ast.ShowStmt) error { output.WriteString(fmt.Sprintf("Unknown element type for: %s\n", name)) } + if ctx.Format == FormatJSON { + // The context is assembled as markdown for a model to read, so JSON + // wraps it rather than re-deriving it: the same envelope shape as + // `describe --json` ({name, type, mdl}), with the text under "context". + // It used to ignore --json and print the markdown bare, which a caller + // that asked for JSON cannot parse. + enc := json.NewEncoder(ctx.Output) + enc.SetIndent("", " ") + return enc.Encode(map[string]any{ + "name": name, + "type": targetType, + "depth": depth, + "context": output.String(), + }) + } + fmt.Fprint(ctx.Output, output.String()) return nil } diff --git a/mdl/executor/cmd_datatransformer.go b/mdl/executor/cmd_datatransformer.go index 9d21c684d..913d08572 100644 --- a/mdl/executor/cmd_datatransformer.go +++ b/mdl/executor/cmd_datatransformer.go @@ -41,7 +41,7 @@ func listDataTransformers(ctx *ExecContext, moduleName string) error { rows = append(rows, []any{qn, modName, dt.Name, dt.SourceType, steps}) } - if len(rows) == 0 { + if len(rows) == 0 && ctx.Format != FormatJSON { fmt.Fprintln(ctx.Output, "No data transformers found.") return nil } diff --git a/mdl/executor/cmd_export_mappings.go b/mdl/executor/cmd_export_mappings.go index eebdd0736..cdbfef5cd 100644 --- a/mdl/executor/cmd_export_mappings.go +++ b/mdl/executor/cmd_export_mappings.go @@ -57,7 +57,7 @@ func listExportMappings(ctx *ExecContext, inModule string) error { rows = append(rows, row{qualifiedName: qn, name: em.Name, schemaSource: src, elementCount: len(em.Elements)}) } - if len(rows) == 0 { + if len(rows) == 0 && ctx.Format != FormatJSON { if inModule != "" { fmt.Fprintf(ctx.Output, "No export mappings found in module %s\n", inModule) } else { diff --git a/mdl/executor/cmd_import_mappings.go b/mdl/executor/cmd_import_mappings.go index 9860d3da0..8ababaf49 100644 --- a/mdl/executor/cmd_import_mappings.go +++ b/mdl/executor/cmd_import_mappings.go @@ -57,7 +57,7 @@ func listImportMappings(ctx *ExecContext, inModule string) error { rows = append(rows, row{qualifiedName: qn, name: im.Name, schemaSource: src, elementCount: len(im.Elements)}) } - if len(rows) == 0 { + if len(rows) == 0 && ctx.Format != FormatJSON { if inModule != "" { fmt.Fprintf(ctx.Output, "No import mappings found in module %s\n", inModule) } else { diff --git a/mdl/executor/cmd_navigation.go b/mdl/executor/cmd_navigation.go index 2dc203b63..1052d87b3 100644 --- a/mdl/executor/cmd_navigation.go +++ b/mdl/executor/cmd_navigation.go @@ -162,7 +162,7 @@ func listNavigation(ctx *ExecContext) error { return mdlerrors.NewBackend("get navigation", err) } - if len(nav.Profiles) == 0 { + if len(nav.Profiles) == 0 && ctx.Format != FormatJSON { fmt.Fprintln(ctx.Output, "No navigation profiles found.") return nil } diff --git a/mdl/executor/cmd_search.go b/mdl/executor/cmd_search.go index 557fd6277..4373c295a 100644 --- a/mdl/executor/cmd_search.go +++ b/mdl/executor/cmd_search.go @@ -88,11 +88,11 @@ func execShowCallers(ctx *ExecContext, s *ast.ShowStmt) error { } targetName := s.Name.String() - fmt.Fprintf(ctx.Output, "\nCallers of %s", targetName) + fmt.Fprintf(ctx.progress(), "\nCallers of %s", targetName) if s.Transitive { - fmt.Fprintln(ctx.Output, " (transitive)") + fmt.Fprintln(ctx.progress(), " (transitive)") } else { - fmt.Fprintln(ctx.Output, "") + fmt.Fprintln(ctx.progress(), "") } var query string @@ -130,11 +130,10 @@ func execShowCallers(ctx *ExecContext, s *ast.ShowStmt) error { } if result.Count == 0 { - fmt.Fprintln(ctx.Output, "(no callers found)") - return nil + return writeEmptyResult(ctx, result.Columns, "(no callers found)") } - fmt.Fprintf(ctx.Output, "Found %d caller(s)\n", result.Count) + fmt.Fprintf(ctx.progress(), "Found %d caller(s)\n", result.Count) outputCatalogResults(ctx, result) return nil } @@ -151,11 +150,11 @@ func execShowCallees(ctx *ExecContext, s *ast.ShowStmt) error { } sourceName := s.Name.String() - fmt.Fprintf(ctx.Output, "\nCallees of %s", sourceName) + fmt.Fprintf(ctx.progress(), "\nCallees of %s", sourceName) if s.Transitive { - fmt.Fprintln(ctx.Output, " (transitive)") + fmt.Fprintln(ctx.progress(), " (transitive)") } else { - fmt.Fprintln(ctx.Output, "") + fmt.Fprintln(ctx.progress(), "") } var query string @@ -193,11 +192,10 @@ func execShowCallees(ctx *ExecContext, s *ast.ShowStmt) error { } if result.Count == 0 { - fmt.Fprintln(ctx.Output, "(no callees found)") - return nil + return writeEmptyResult(ctx, result.Columns, "(no callees found)") } - fmt.Fprintf(ctx.Output, "Found %d callee(s)\n", result.Count) + fmt.Fprintf(ctx.progress(), "Found %d callee(s)\n", result.Count) outputCatalogResults(ctx, result) return nil } @@ -214,7 +212,7 @@ func execShowReferences(ctx *ExecContext, s *ast.ShowStmt) error { } typed := s.Name.String() - fmt.Fprintf(ctx.Output, "\nReferences to %s\n", typed) + fmt.Fprintf(ctx.progress(), "\nReferences to %s\n", typed) // A widget's TargetName is stored SHOUTED (COMBOBOX) while MDL keywords are // written in lower case, so an exact-only match answers the natural spelling @@ -236,11 +234,10 @@ func execShowReferences(ctx *ExecContext, s *ast.ShowStmt) error { } if result.Count == 0 { - fmt.Fprintln(ctx.Output, "(no references found)") - return nil + return writeEmptyResult(ctx, result.Columns, "(no references found)") } - fmt.Fprintf(ctx.Output, "Found %d reference(s)\n", result.Count) + fmt.Fprintf(ctx.progress(), "Found %d reference(s)\n", result.Count) outputCatalogResults(ctx, result) return nil } @@ -258,7 +255,7 @@ func execShowImpact(ctx *ExecContext, s *ast.ShowStmt) error { } typed := s.Name.String() - fmt.Fprintf(ctx.Output, "\nImpact analysis for %s\n", typed) + fmt.Fprintf(ctx.progress(), "\nImpact analysis for %s\n", typed) targetName, loose := resolveReferenceTarget(ctx, typed) reportResolvedTarget(ctx, typed, targetName, loose) @@ -277,8 +274,7 @@ func execShowImpact(ctx *ExecContext, s *ast.ShowStmt) error { } if result.Count == 0 { - fmt.Fprintln(ctx.Output, "(no impact - element is not referenced)") - return nil + return writeEmptyResult(ctx, result.Columns, "(no impact - element is not referenced)") } // Group by type for summary @@ -291,13 +287,13 @@ func execShowImpact(ctx *ExecContext, s *ast.ShowStmt) error { } } - fmt.Fprintf(ctx.Output, "\nSummary:\n") + fmt.Fprintf(ctx.progress(), "\nSummary:\n") for t, count := range typeCounts { - fmt.Fprintf(ctx.Output, " %s: %d\n", t, count) + fmt.Fprintf(ctx.progress(), " %s: %d\n", t, count) } - fmt.Fprintln(ctx.Output) + fmt.Fprintln(ctx.progress()) - fmt.Fprintf(ctx.Output, "Found %d affected element(s)\n", result.Count) + fmt.Fprintf(ctx.progress(), "Found %d affected element(s)\n", result.Count) outputCatalogResults(ctx, result) return nil diff --git a/mdl/executor/cmd_widgets.go b/mdl/executor/cmd_widgets.go index 151bc1c6c..39f3222c3 100644 --- a/mdl/executor/cmd_widgets.go +++ b/mdl/executor/cmd_widgets.go @@ -55,8 +55,10 @@ func execShowWidgets(ctx *ExecContext, s *ast.ShowWidgetsStmt) error { // Output results as table if result.Count == 0 { - fmt.Fprintln(ctx.Output, "No widgets found matching the criteria") - return nil + return writeEmptyResult(ctx, result.Columns, "No widgets found matching the criteria") + } + if ctx.Format == FormatJSON { + return writeResult(ctx, &TableResult{Columns: result.Columns, Rows: result.Rows}) } // Print header diff --git a/mdl/executor/exec_context.go b/mdl/executor/exec_context.go index 7a5e97a65..8425b04b9 100644 --- a/mdl/executor/exec_context.go +++ b/mdl/executor/exec_context.go @@ -34,6 +34,12 @@ type ExecContext struct { // Output is the writer for user-visible output (with line-limit guard). Output io.Writer + // Diagnostics receives warnings about the answer rather than the answer + // itself — "this result is incomplete because…" — so they never land in a + // payload a caller parses (search --format json). Nil means os.Stderr; use + // diagnostics() rather than reading the field. + Diagnostics io.Writer + // describeQualifyAttrs is set by DESCRIBE PAGE while it reads the widgets // inside a data container whose flow cannot be resolved: no entity is in // scope there, so an attribute binding keeps its stored Module.Entity.Attr @@ -135,6 +141,31 @@ type ExecContext struct { ScriptDir string } +// diagnostics returns the writer for warnings about a result (see the +// Diagnostics field): the configured one, or os.Stderr. +func (ctx *ExecContext) diagnostics() io.Writer { + if ctx.Diagnostics != nil { + return ctx.Diagnostics + } + return os.Stderr +} + +// progress returns the writer for commentary about a run rather than its +// answer: "Connected to:", catalog load/build progress, a result header or +// count, "(no references found)". For a person that commentary is part of the +// reading, so in text mode it stays on Output exactly as before. When Output +// carries a JSON payload it goes to diagnostics() instead — every byte on +// Output is then handed to a parser, and one "Connected to:" line ahead of the +// array is enough to make `mxcli refs --json | jq .` fail. +// +// Route progress here, not to Output, in any code a JSON-mode command reaches. +func (ctx *ExecContext) progress() io.Writer { + if ctx.Format == FormatJSON { + return ctx.diagnostics() + } + return ctx.Output +} + // ResolveScriptRelative turns a path written inside an MDL script into an // absolute one: relative to the script's own directory when that is known, and // to the working directory otherwise. diff --git a/mdl/executor/executor_connect.go b/mdl/executor/executor_connect.go index 6e0acf9b6..c17f7bba5 100644 --- a/mdl/executor/executor_connect.go +++ b/mdl/executor/executor_connect.go @@ -37,7 +37,7 @@ func execConnect(ctx *ExecContext, s *ast.ConnectStmt) error { // Display connection info with version pv := ctx.Backend.ProjectVersion() if !ctx.Quiet { - fmt.Fprintf(ctx.Output, "Connected to: %s (Mendix %s)\n", s.Path, pv.ProductVersion) + fmt.Fprintf(ctx.progress(), "Connected to: %s (Mendix %s)\n", s.Path, pv.ProductVersion) } if ctx.Logger != nil { ctx.Logger.Connect(s.Path, pv.ProductVersion, pv.FormatVersion) diff --git a/mdl/executor/format.go b/mdl/executor/format.go index 743edb256..2d3d086bb 100644 --- a/mdl/executor/format.go +++ b/mdl/executor/format.go @@ -36,6 +36,20 @@ func writeResult(ctx *ExecContext, r *TableResult) error { return nil } +// writeEmptyResult reports a query that matched nothing. In text mode that is +// the sentence a person expects ("(no references found)"); in JSON mode it is +// an empty array, with the sentence moved to the progress stream. An empty +// answer is exactly the case a caller must be able to parse — a sentence there +// reads as a corrupt payload rather than as "no". +func writeEmptyResult(ctx *ExecContext, columns []string, message string) error { + if ctx.Format != FormatJSON { + fmt.Fprintln(ctx.Output, message) + return nil + } + fmt.Fprintln(ctx.progress(), message) + return writeResultJSON(ctx, &TableResult{Columns: columns}) +} + // writeResultTable renders a TableResult as a pipe-delimited markdown table. func writeResultTable(ctx *ExecContext, r *TableResult) { if len(r.Columns) == 0 { diff --git a/mdl/executor/progress_stream_test.go b/mdl/executor/progress_stream_test.go new file mode 100644 index 000000000..ce7f2a916 --- /dev/null +++ b/mdl/executor/progress_stream_test.go @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" +) + +// Progress is commentary about a run. In text mode it is part of what a person +// reads and stays on Output; in JSON mode Output is a payload and progress moves +// to the diagnostics stream. The end-to-end version of this, across every query +// subcommand, is cmd/mxcli's TestJSONFlagKeepsStdoutPureJSON. +func TestProgressFollowsTheOutputFormat(t *testing.T) { + var out, diag bytes.Buffer + + text := &ExecContext{Output: &out, Diagnostics: &diag, Format: FormatTable} + if text.progress() != &out { + t.Error("text mode: progress must stay on Output, where a person has always read it") + } + + js := &ExecContext{Output: &out, Diagnostics: &diag, Format: FormatJSON} + if js.progress() != &diag { + t.Error("JSON mode: progress must go to Diagnostics, not into the payload") + } +} + +// An empty answer must be parseable in JSON mode: "[]", with the sentence on the +// diagnostics stream. In text mode it is the sentence, as before. +func TestWriteEmptyResult(t *testing.T) { + var out, diag bytes.Buffer + ctx := &ExecContext{Output: &out, Diagnostics: &diag, Format: FormatJSON} + if err := writeEmptyResult(ctx, []string{"SourceName"}, "(no references found)"); err != nil { + t.Fatal(err) + } + if got := strings.TrimSpace(out.String()); got != "[]" { + t.Errorf("JSON mode payload = %q, want []", got) + } + if !strings.Contains(diag.String(), "(no references found)") { + t.Errorf("JSON mode: the message should still reach diagnostics, got %q", diag.String()) + } + + out.Reset() + diag.Reset() + ctx.Format = FormatTable + if err := writeEmptyResult(ctx, nil, "(no references found)"); err != nil { + t.Fatal(err) + } + if got := strings.TrimSpace(out.String()); got != "(no references found)" { + t.Errorf("text mode output = %q, want the sentence unchanged", got) + } + if diag.Len() != 0 { + t.Errorf("text mode wrote to diagnostics: %q", diag.String()) + } +} diff --git a/mdl/executor/reference_target.go b/mdl/executor/reference_target.go index a458c3f8f..9a14aafd0 100644 --- a/mdl/executor/reference_target.go +++ b/mdl/executor/reference_target.go @@ -64,5 +64,5 @@ func reportResolvedTarget(ctx *ExecContext, typed, resolved string, matchedLoose if !matchedLoosely || ctx == nil || ctx.Output == nil { return } - fmt.Fprintf(ctx.Output, "(matched %s)\n", strings.TrimSpace(resolved)) + fmt.Fprintf(ctx.progress(), "(matched %s)\n", strings.TrimSpace(resolved)) } From a37ccdf35fe46a7208a5ba5f5a48fd7c86fac65a Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:26:46 +0000 Subject: [PATCH 10/13] docs(findings): record the --json purity recurrence Co-Authored-By: Claude Opus 5.5 --- .claude/skills/fix-issue/findings/cmd-mxcli.jsonl | 1 + docs-wiki/bug-patterns/cli-contract-defects.md | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index 6b1439dcf..540422208 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -129,3 +129,4 @@ {"area": "cmd/mxcli", "date": "2026-09-25", "symptom": "Shipped Starlark rules CONV009, CONV010, QUAL001, QUAL003, QUAL004 and CUSTOM002 report nanoflows and rules as microflows: \"Microflow 'NF_Foo' has 30 activities\", and documentType \"Microflow\" in the JSON and report output.", "cause": "microflows() yields all three flow flavours (one catalog table, MicroflowType MICROFLOW/NANOFLOW/RULE), and each rule hardcoded document_type=\"Microflow\" and a \"Microflow '...'\" message. The Go rules had already been fixed with Microflow.DocumentNoun() after MPR002 called a rule a microflow, but that method was never exposed to Starlark, so every Starlark rule had to re-derive it and none did.", "file": "mdl/linter/starlark.go, .claude/lint-rules/{conv009_max_microflow_objects,conv010_act_microflow_content,example_microflow,long_microflows,mccabe_complexity,orphaned_elements}.star, mdl/linter/starlark_flow_noun_test.go", "insight": "When a Go-side fix lives in a method, check whether the Starlark projection can reach it; a fix Starlark cannot call is a fix for half the rules. Exposed as document_noun / document_noun_title on the microflow struct, and the test runs every shipped rule over a fixture with one flow per flavour, requiring a finding on each (else vacuous) plus a guard that any new `for x in microflows()` rule joins the list. Skip the wrong turn of excluding nanoflows/rules from QUAL004: rule calls from decisions ARE emitted as 'call' refs (builder_references.go collectRuleCalls), so only the label was wrong.", "refs": "mendixlabs/mxcli#1178"} {"area": "cmd/mxcli", "date": "2026-09-25", "symptom": "Starlark rule API docs name microflow_type values the linter never returns: the write-lint-rules skill said \"microflow\"/\"nanoflow\", example_microflow.star (copied into every project by mxcli init) said \"Microflow\"/\"Nanoflow\", only mccabe_complexity.star had \"MICROFLOW\"/\"NANOFLOW\", and none listed \"RULE\". The skill's entity table also omitted has_created_date/has_changed_date/has_owner/has_changed_by.", "cause": "LintContext.Microflows() passes the catalog's MicroflowType through raw (MICROFLOW/NANOFLOW/RULE) while Entities() CASE-normalizes EntityType to TitleCase, so the two adjacent iterators have opposite conventions and each doc author guessed. Nothing tied the documented literals to what the API emits, so two passes over the same skill file (59db6e7b, #1165) fixed instances and left this one.", "file": ".claude/skills/mendix/write-lint-rules/SKILL.md, .claude/lint-rules/example_microflow.star, .claude/lint-rules/mccabe_complexity.star, mdl/linter/starlark_documented_values_test.go", "insight": "Fix the class, not the row: observe the emitted field names and enum values by running a Starlark rule (dir(e), e.entity_type) over a fixture holding every stored kind, then assert the skill tables, the shipped rules' `# .field - ...` headers, and every `.entity_type/.microflow_type ==` comparison in shipped rules against that observed set. Equality, not subset, for the docs -- an undocumented value (RULE) is a flavour a rule silently mistreats. Normalizing MicroflowType instead would have broken every user rule already comparing \"MICROFLOW\" correctly.", "refs": "mendixlabs/mxcli#1178, mendixlabs/mxcli#1164"} {"area": "cmd/mxcli", "date": "2026-09-26", "symptom": "The write-lint-rules skill, which Starlark rules are written from, omitted seven query functions (java_actions, documents, documentable_elements, navigation_targets, queues, module_cycles, database_connections) plus get_option() and struct(), and fields on scheduled_event (repeat, on_overlap, time_zone) and project_security (anonymous_user_role). Four of the missing functions are used by shipped rules.", "cause": "Each builtin and field was added in code with its own PR and a docs-site paragraph at most; nothing compared the skill to buildPredeclared() or to the struct keys, so every API addition drifted from the skill by default. #1178's test pinned only the entity and microflow tables.", "file": ".claude/skills/mendix/write-lint-rules/SKILL.md, mdl/linter/starlark_skill_coverage_test.go", "insight": "Cover the whole surface at once rather than the table that was reported: an internal test ranges over (&StarlarkRule{}).buildPredeclared() for builtins and go/ast-parses every starlarkstruct.FromStringDict(starlark.String(name), dict) call in the package for struct fields (literal dicts and locally-built ones like ppDict), then requires each struct's skill table to match exactly. Structs documented elsewhere are named in explicit maps (helper params, inline struct{...} in a function row, shared permission table) so an exemption is a visible decision. The quick one-off Python diff over-reported (nested password_policy, permissions_for's entity_name) -- the test encodes those layouts instead of guessing.", "refs": "mendixlabs/mxcli#1178"} +{"area": "cmd/mxcli", "date": "2026-09-26", "symptom": "`--json` output did not parse. `mxcli refs|callers|callees|impact|structure|show|search … --json` and `mxcli -c \"select … from CATALOG.x\" --json` printed \"Connected to:\", catalog load/build progress (`✓ Table: N`, 40+ lines on a cold cache), a \"References to X\" header and a \"Found N\" count ahead of the array; an empty answer was the sentence \"(no references found)\" / \"(no results)\" instead of []; `context` ignored --json and printed markdown; `check --format json` wrote its document to STDERR (one per phase), leaving only executor chatter on stdout; `show widgets|data transformers|import/export mappings|catalog status` had no JSON path for empty or at all; `diff --json` silently printed a text diff. Found by an agent-orientation audit on a 140MB app (Evora Factory Management)", "cause": "The executor has ONE writer, ctx.Output, and it carries both the payload and every status line. The #904 fix (progressSink) only works when the payload is written by the COMMAND (lint/report format their own result to os.Stdout, so the executor's writer can be pointed at stderr); for every query subcommand the payload is written by the executor itself, through the same ctx.Output as the chatter, so no choice of writer at the cmd layer can separate them. The split has to be inside the executor, per line, by what the line is", "file": "`mdl/executor/exec_context.go` (`Diagnostics` field + `diagnostics()`, identical to ako/mxcli#716, plus `progress()`: Output in text mode, diagnostics() in JSON mode), `mdl/executor/format.go` (`writeEmptyResult`), `mdl/executor/cmd_catalog.go` (connect/load/build/warn/count lines, `writeCatalogStatusJSON`), `mdl/executor/cmd_search.go`, `cmd_context.go`, `cmd_widgets.go`, `executor_connect.go`, `reference_target.go`; `cmd/mxcli/cmd_check.go` (one structured document on stdout via `finish`), `cmd/mxcli/cmd_query.go` (search: `--json` canonical, `--format json` deprecated alias sets the executor format), `cmd/mxcli/output_stream.go` (`refuseJSONFlag`, used by diff/diff-local); tests `cmd/mxcli/json_output_purity_test.go`, `mdl/executor/progress_stream_test.go`", "insight": "**Classify each line by what it IS, not where it is printed**: an answer goes to Output, commentary about the run goes to `ctx.progress()`. Text mode is byte-for-byte unchanged because progress() IS Output there — assert that too (`TestTextModeKeepsProgressOnStdout`), or the next fix 'cleans' interactive output by sending it all to stderr. **The empty case is where JSON contracts break**: every handler that early-returns a sentence on zero rows must go through writeEmptyResult (or the existing `&& ctx.Format != FormatJSON` idiom) — the sweep found 8 such sites and most list handlers already had the idiom, so grep for the ones that do not. **--json is a ROOT persistent flag**, so every subcommand accepts it; a command with no JSON output must refuse it (stderr, exit 2), since ignoring it hands text to a parser. **Test in a child process** (reuse TestRunMainHelper): stdout and stderr are then genuinely separate, error paths ending in os.Exit are observable, and no cobra flag state leaks between table rows. Proof: forcing progress() to return Output fails 18/18 --json rows, the search alias and the error test; reverting cmd_check.go fails all five check rows. Left alone: the 10,000-line output guard counts pretty-printed JSON lines, so `show widgets --json` on Evora emits the complete document and then exits 1 'output line limit exceeded'", "refs": ["#904", "ako/mxcli#716"]} diff --git a/docs-wiki/bug-patterns/cli-contract-defects.md b/docs-wiki/bug-patterns/cli-contract-defects.md index 80f95ca85..67205ece2 100644 --- a/docs-wiki/bug-patterns/cli-contract-defects.md +++ b/docs-wiki/bug-patterns/cli-contract-defects.md @@ -39,7 +39,11 @@ promise, and the cheapest guard is that its effect has exactly one call site. **stdout is a data channel.** `--format json | jq` failed because progress lines shared the stream. Anything with a machine-readable format has to keep -diagnostics on stderr. +diagnostics on stderr. It came back across the whole query family because +choosing the executor's writer only works when the *command* prints the +payload; where the executor prints it, the split has to be made per line inside +the executor (answer vs. commentary) — and the empty result, the error, and a +command that silently ignores the root `--json` flag are where it breaks. **Path handling is where portability bugs live.** Windows backslashes mangled by escape processing, a relative `-p` rejected by a downstream tool with its own From 35bc2a470610b4b4d1425f4826ea6555931f8817 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 27 Sep 2026 06:59:36 +0000 Subject: [PATCH 11/13] roundtrip: strike putget for SUB_Feedback_PostToAppInsights, fixed by trimming expression whitespace Co-Authored-By: Claude Opus 5.5 --- mdl/roundtrip/allowlist_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mdl/roundtrip/allowlist_test.go b/mdl/roundtrip/allowlist_test.go index 1373f0889..9d1d35b2d 100644 --- a/mdl/roundtrip/allowlist_test.go +++ b/mdl/roundtrip/allowlist_test.go @@ -178,7 +178,7 @@ var knownFailures = map[string]knownFailure{ "microflow FeedbackModule.ConvertBase64String": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, "microflow FeedbackModule.ConvertUUIDToURL": {laws: []law{lawGetPut}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, "microflow FeedbackModule.PopulateUserAttributes": {laws: []law{lawGetPut}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, - "microflow FeedbackModule.SUB_Feedback_PostToAppInsights": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, + "microflow FeedbackModule.SUB_Feedback_PostToAppInsights": {laws: []law{lawGetPut}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A); putget fixed by #718 (expression whitespace)"}, "microflow FeedbackModule.SUB_Feedback_Sanitize": {laws: []law{lawGetPut}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, "microflow FeedbackModule.SUB_Feedback_SendToServer": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, "microflow FeedbackModule.VAL_Feedback": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, From 034185f8ddf3f33f4ff14b4b32c6c21fe4b7b599 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 27 Sep 2026 07:00:01 +0000 Subject: [PATCH 12/13] merge: drop the diagnostics() helper duplicated by #716 and #719 Co-Authored-By: Claude Opus 5.5 --- mdl/executor/exec_context.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/mdl/executor/exec_context.go b/mdl/executor/exec_context.go index a6be741ae..fa0220ebb 100644 --- a/mdl/executor/exec_context.go +++ b/mdl/executor/exec_context.go @@ -158,15 +158,6 @@ func (ctx *ExecContext) diagnostics() io.Writer { return os.Stderr } -// diagnostics returns the writer for warnings about a result (see the -// Diagnostics field): the configured one, or os.Stderr. -func (ctx *ExecContext) diagnostics() io.Writer { - if ctx.Diagnostics != nil { - return ctx.Diagnostics - } - return os.Stderr -} - // progress returns the writer for commentary about a run rather than its // answer: "Connected to:", catalog load/build progress, a result header or // count, "(no references found)". For a person that commentary is part of the From af0dbb29ea3a33002075a5cc558d09e2dbed3d3e Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 27 Sep 2026 07:02:36 +0000 Subject: [PATCH 13/13] test: #720 made the no-references message more specific; match its prefix Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/json_output_purity_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/mxcli/json_output_purity_test.go b/cmd/mxcli/json_output_purity_test.go index efe58f4a1..8cd37e90b 100644 --- a/cmd/mxcli/json_output_purity_test.go +++ b/cmd/mxcli/json_output_purity_test.go @@ -206,7 +206,7 @@ func TestTextModeKeepsProgressOnStdout(t *testing.T) { want []string // lines a person has always seen on stdout }{ {[]string{"refs", "System.User"}, []string{"Connected to:", "Building catalog", "References to System.User"}}, - {[]string{"refs", "Nope.Nothing"}, []string{"Connected to:", "(no references found)"}}, + {[]string{"refs", "Nope.Nothing"}, []string{"Connected to:", "(no references found"}}, {[]string{"callers", "System.User"}, []string{"Connected to:", "Callers of System.User", "(no callers found)"}}, {[]string{"impact", "System.User"}, []string{"Impact analysis for System.User", "Summary:"}}, {[]string{"context", "System.User"}, []string{"Connected to:", "## Context: System.User"}},