diff --git a/.claude/lint-rules/missing_documentation.star b/.claude/lint-rules/missing_documentation.star index da8697dcb..745cf2da0 100644 --- a/.claude/lint-rules/missing_documentation.star +++ b/.claude/lint-rules/missing_documentation.star @@ -1,66 +1,249 @@ # Starlark Lint Rule: Missing Documentation # -# This rule checks that entities and microflows have documentation. -# Good documentation helps with maintainability and onboarding new developers. +# Undocumented model elements are invisible to `mxcli check` and to the build — +# nothing fails, so nothing reminds you. This rule is the reminder, and it +# covers every document type a user authors, not just the domain model. # -# Checks: -# - Entities should have a description explaining their purpose -# - Microflows should have a description explaining what they do +# Documents swept generically (one option each, all default True): +# Module, Entity, Page, Snippet, BuildingBlock, Layout, Enumeration, +# JavaScriptAction, ImageCollection, DataTransformer, Workflow, +# BusinessEventService, RestClient, PublishedRestService, Constant, +# JsonStructure, ImportMapping, ExportMapping # -# Entity properties: -# .description - Documentation text +# Handled separately, because they carry exemptions or children a uniform +# sweep cannot express: +# Microflow .description (nanoflows and trivial flows exempt) +# JavaAction .documentation +# JavaAction params .description <- the one Studio Pro shows a CALLER # -# Microflow properties: -# .description - Documentation text +# Members, off by default purely because of volume: +# Attribute .description +# Association .description +# +# Why Java action parameters default ON while attributes default OFF: an action +# has a handful of parameters and Studio Pro renders each description in the +# dialog where someone wires up the call — an undocumented parameter is a blank +# field next to a name like `pInput` at exactly the moment a caller has to +# decide what to pass. A domain model has hundreds of attributes and +# associations, so the same check there is a wall of text rather than a signal. +# +# Every kind is individually switchable; see the table in _DOC_KINDS and the +# options listed under docs-site/src/tools/starlark-rules.md. RULE_ID = "QUAL002" RULE_NAME = "Missing Documentation" -DESCRIPTION = "Entities and microflows should have documentation describing their purpose" +DESCRIPTION = "Model elements should have documentation describing their purpose" CATEGORY = "quality" SEVERITY = "info" +# kind (as emitted by documentable_elements) -> (option, noun, suggestion) +# +# A new Mendix document type is covered by adding a row in Go's +# documentableSources and a row here — not by writing another loop. +_DOC_KINDS = { + "Module": ( + "check_modules", + "Module", + "Document what the module is for: it is the first thing a newcomer opens.", + ), + "Entity": ( + "check_entities", + "Entity", + "Add a description explaining the entity's purpose and what data it represents.", + ), + "Page": ( + "check_pages", + "Page", + "Describe what the page shows and who reaches it.", + ), + "Snippet": ( + "check_snippets", + "Snippet", + "Describe what the snippet renders and what context it expects, since it is reused across pages.", + ), + "BuildingBlock": ( + "check_building_blocks", + "Building block", + "Describe what the building block is for: it exists to be dropped in by someone who did not write it.", + ), + "Layout": ( + "check_layouts", + "Layout", + "Describe the layout's intended use and its placeholders.", + ), + "Enumeration": ( + "check_enumerations", + "Enumeration", + "Describe what the enumeration models, especially where the values map to something external.", + ), + "JavaScriptAction": ( + "check_javascript_actions", + "JavaScript action", + "Document what the action does and what it returns. Like a Java action, its body is code the model cannot show a reader.", + ), + "ImageCollection": ( + "check_image_collections", + "Image collection", + "Describe what the collection is for and where its images are used.", + ), + "DataTransformer": ( + "check_data_transformers", + "Data transformer", + "Describe the transformation applied and the shape it expects.", + ), + "Workflow": ( + "check_workflows", + "Workflow", + "Describe the process the workflow models and who its user tasks are for.", + ), + "BusinessEventService": ( + "check_business_event_services", + "Business event service", + "Document the events published or consumed, since other applications depend on them.", + ), + "RestClient": ( + "check_rest_clients", + "REST client", + "Document which external service is consumed and what it is used for.", + ), + "PublishedRestService": ( + "check_published_rest_services", + "Published REST service", + "Document the contract: this is the description external consumers read.", + ), + "Constant": ( + "check_constants", + "Constant", + "Describe what the constant configures and what a valid value looks like — it is set per environment by someone who cannot see the code.", + ), + "JsonStructure": ( + "check_json_structures", + "JSON structure", + "Note which payload the structure was captured from.", + ), + "ImportMapping": ( + "check_import_mappings", + "Import mapping", + "Describe the source payload and what it maps onto.", + ), + "ExportMapping": ( + "check_export_mappings", + "Export mapping", + "Describe the target payload and what it is produced for.", + ), + "Association": ( + # Off by default with attributes: a real domain model has as many + # associations as entities, and none of them are documented. + "check_associations", + "Association", + "Add a description, or switch this off with `check_associations: false` if the names are self-describing here.", + ), +} + +# Kinds whose option defaults to False. Everything else defaults to True. +_OFF_BY_DEFAULT = {"check_associations": True} + +def _blank(text): + """True when a documentation field is absent or whitespace-only.""" + return not text or text.strip() == "" + +def _flag(violations, module, doc_type, doc_name, message, suggestion): + violations.append(violation( + message = message, + location = location( + module = module, + document_type = doc_type, + document_name = doc_name, + ), + suggestion = suggestion, + )) + def check(): - """ - Check that entities and microflows have documentation. - """ violations = [] - # Check entities - for entity in entities(): - if not entity.description or entity.description.strip() == "": - loc = location( - module=entity.module_name, - document_type="Entity", - document_name=entity.qualified_name - ) - v = violation( - message="Entity '{}' has no documentation.".format(entity.name), - location=loc, - suggestion="Add a description explaining the entity's purpose and what data it represents." + # ---- every document type, one sweep ------------------------------------- + for el in documentable_elements(): + entry = _DOC_KINDS.get(el.kind) + if entry == None: + # A kind Go knows about but this table does not. Staying silent is + # right: a rule inventing a message for an element it cannot + # describe is worse than not reporting it. + continue + option, noun, suggestion = entry + if not get_option(option, not _OFF_BY_DEFAULT.get(option, False)): + continue + if _blank(el.description): + _flag( + violations, + el.module_name, + el.kind, + el.qualified_name, + "{} '{}' has no documentation.".format(noun, el.name), + suggestion, ) - violations.append(v) - # Check microflows (skip nanoflows as they're often simple) - for mf in microflows(): - # Only check microflows, not nanoflows - if mf.microflow_type != "MICROFLOW": - continue + # ---- microflows: exempt nanoflows and trivial flows --------------------- + if get_option("check_microflows", True): + # Nanoflows are excluded: they are usually a couple of client-side steps + # whose name says everything a description would. + min_activities = get_option("min_activities", 3) + for mf in microflows(): + if mf.microflow_type != "MICROFLOW": + continue + if mf.activity_count < min_activities: + continue + if _blank(mf.description): + _flag( + violations, + mf.module_name, + "Microflow", + mf.qualified_name, + "Microflow '{}' has no documentation.".format(mf.name), + "Add a description explaining what this microflow does and when it should be called.", + ) - # Skip very simple microflows (1-2 activities) - if mf.activity_count <= 2: - continue + # ---- Java actions and their parameters ---------------------------------- + check_actions = get_option("check_java_actions", True) + check_params = get_option("check_java_action_params", True) + if check_actions or check_params: + for ja in java_actions(): + if check_actions and _blank(ja.documentation): + _flag( + violations, + ja.module_name, + "JavaAction", + ja.qualified_name, + "Java action '{}' has no documentation.".format(ja.name), + "Add documentation explaining what the action does, and what it returns. " + + "Unlike a microflow, its body is Java that the model cannot show a reader.", + ) + if not check_params: + continue + for p in ja.parameters: + if _blank(p.description): + _flag( + violations, + ja.module_name, + "JavaAction", + ja.qualified_name, + "Java action parameter '{}.{}' has no description.".format(ja.name, p.name), + "Add a description: Studio Pro shows it to whoever wires up the call, " + + "where the parameter name is all they otherwise have to go on.", + ) - if not mf.description or mf.description.strip() == "": - loc = location( - module=mf.module_name, - document_type="Microflow", - document_name=mf.qualified_name - ) - v = violation( - message="Microflow '{}' has no documentation.".format(mf.name), - location=loc, - suggestion="Add a description explaining what this microflow does and when it should be called." - ) - violations.append(v) + # ---- entity attributes (off by default: high volume) -------------------- + if get_option("check_attributes", False): + for entity in entities(): + for attr in attributes_for(entity.qualified_name): + if _blank(attr.description): + _flag( + violations, + entity.module_name, + "Entity", + entity.qualified_name, + "Attribute '{}.{}' has no documentation.".format(entity.name, attr.name), + "Add a description, or switch this off with `check_attributes: false` if " + + "attribute names are self-describing in this project.", + ) return violations diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 85535a966..1068db708 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -427,3 +427,26 @@ extracting `OffsetExpression`/`LimitExpression`. | `create odata client` with credentials given as constants (`HttpUsername: '@Module.ApiUser'`) still gets HTTP 401 and an empty client, after the fix that made literal credentials work. Sharpened by the same release making a constant `ServiceUrl` mandatory — the shape the tool insists on is the shape whose credentials it will not read | `resolveCredential` trusted the visitor's isLiteral flag. `'@Module.ApiUser'` **is** a STRING_LITERAL, so the flag said "literal" and the previous code sent the fifteen characters `@Module.ApiUser` as the username — and the unresolved-credential note did not fire either, because as far as the code knew nothing was unresolved | `mdl/executor/cmd_odata.go` (`resolveCredential`, `constantReference`, `designTimeConstants`) | **A syntactic classification is not a semantic one.** The visitor can say "this was a quoted string"; only the executor can say "this quoted string names a constant". Any flag of the form isLiteral needs the consumer to ask what the literal *contains* before treating it as a value. **The fix is to resolve, not to refuse**: a constant's design-time default is exactly what Studio Pro sends on the same fetch, so reading it is the value rather than a workaround — and mxcli already has the project open. Three spellings must all work (`'v'`, `@M.C`, `'@M.C'`); a dotted literal like a password containing a dot must not be mistaken for a reference. Tests `cmd_odata_metadata_auth_test.go`. mxcli-formula1 #23 follow-up | | An app themed dark still shows light-mode drop shadows under the datagrid's filter-operator popover and dropdown filter lists | The generated widget layer re-pointed `.column-selectors` but not the four rules in `_datagrid-filters.scss` that bake the same two-layer shadow. Each already takes its *background* from `--bg-color-secondary`, so Atlas re-colours the panel and leaves the shadow — which is why it reads as a partial fix rather than an untouched widget | `cmd/mxcli/theme/assets/*/files/theme/web/_mxcli-widgets.scss` | **Read the selectors out of the shipped `themesource/`, not the bug report** — the fourth here is `.dropdown-container .dropdown-list`, nested under a parent the report did not name. **Verify in the compiled CSS, never the source** (the §33 rule): apply the theme to a real project, run `mxbuild --target=deploy --java-home=… --java-exe-path=…`, then grep `theme-cache/web/theme.compiled.css` and check the *line number* — the fix must land after the widget module's own rule (30794 vs 27765 here) or the cascade eats it. A theme-cache file older than the SCSS you just wrote is a stale compile, and mtime is the cheapest way to catch it. mxcli-formula1 #33 / suggested issue 4 | | A `MOVE` cannot be confirmed and a module's layout cannot be reviewed: `SHOW STRUCTURE` groups by document type at every depth and never names a folder, `DESCRIBE` answers for one document at a time, so checking where things ended up means opening the `.mpr` as SQLite | The read side of folders was simply never built. `MOVE`/`DROP FOLDER` write containment; nothing read it back | New `mdl/executor/cmd_list_folders.go` + grammar (`FOLDERS` lexer token, `showOrList FOLDERS (IN …)?` in `MDLCatalog.g4`, `FOLDERS` added to the `keyword` rule so it stays usable as an identifier), `mdl/ast/ast_query.go`, `mdl/visitor/visitor_query.go`, `mdl/executor/executor_query.go` | **A layout listing must show what is *not* there**: empty folders (`[0]`) and documents still at the module root, or it cannot be diffed against an intended layout — that is the whole use. Documents are indexed by `ContainerID` across every list call the backend offers, each best-effort, so a backend that cannot answer one kind yields a listing missing that kind rather than no listing. **Do not stub the hierarchy in the test** — `mkHierarchy` populates `moduleNames` but not `folderNames`, so `BuildFolderPath` returns `""` and every folder silently collapses into the module root, which looks exactly like the bug. Build it from the mock's `ListModules`/`ListUnits`/`ListFolders`, as `getHierarchy` does. Tests `cmd_list_folders_test.go`, example in `18-folder-examples.mdl`. mxcli-formula1 issue #2 | +| Every page in a locally-run app is a black screen. `mxcli check` passes, the build reports success, the runtime log is quiet, `curl /` returns 200 with a valid HTML shell, and every OData service answers — only a browser sees it. It also comes and goes between runs of the same command at the same version | `run --local` bundles the browser client at step 5b, then the runtime boot runs Gradle `clean-custom-classes compile package`, whose package pass repopulates `deployment/web` and takes `dist/` with it — deleting the bundle 51 seconds after the same command wrote it. It only bites when Gradle has work to do (a new Java action, a full recompile), which is why the app boots fine for weeks and then stops | `cmd/mxcli/docker/webclient.go` (`WebClientBundled`, `EnsureWebClientBundle`, `ReportLostWebClientBundle`), `cmd/mxcli/docker/runlocal.go` (step 6b, after the boot), `cmd/mxcli/docker/localapp.go` (headless boots warn instead of paying ~30s) | **A pre-condition established before a step that rewrites the same directory is not a post-condition** — verify after, not before. The guard is a `stat` when the bundle survived, which is what makes it affordable at every boot; re-ordering the bundle to after the boot instead would leave the app reachable-but-blank for ~30s on every cold start. **`curl /` returning 200 is not evidence the app renders**: the shell is served by the runtime, the client by a file the shell references. The one-line check is `curl -o /dev/null -w '%{http_code}' /dist/index.js`. **Do not silently pay for a repair on a path that does not need it** — `test --local` destroys the bundle too, but tests are headless, so it prints the loss and the remedy rather than spending 30s on a two-second loop. Tests `webclient_bundle_test.go`; both controls run (guard never fires → the wipe test fails; guard always fires → the survivor test fails). mxcli-formula1 §35 | +| `DESCRIBE PAGE` reports a drill-down button as `linkbutton b (Caption: 'Weekend', Action: show_page Mod.Page)` — the `(Race: $currentObject)` argument is gone. The model is fine (`mx check` is clean, and an unmapped required page parameter is a consistency error, so it could not have built), but the description reads as a diagnosis mid-hunt and costs cycles fixing a button that was correct | mxcli deliberately writes `ParameterMappings` as an empty array for a page action, because Studio Pro infers the row object from the enclosing widget and rejects an explicit `$currentObject` Argument as CE0115 (#296). That decision was right; its other half was missing. `renderClientActionMDL` read only explicit mappings, and the writer's comment *asserted* DESCRIBE recovered the implicit one — it never did | `mdl/executor/cmd_pages_describe_output.go` (`pageActionParameters`, `targetPageParameterNames`), stale comment corrected in `sdk/mpr/writer_widgets_action.go` | **When a writer stores something implicitly, the reader owes it an explicit reconstruction — and a comment claiming the reader already does is worth nothing until a test says so.** Recover from the *target page's* declared parameters, not from the action, since that is where the information actually lives. Guard both ends: an explicit mapping still wins (recovery fills a gap, it does not override Studio Pro), and an unresolvable page yields no arguments rather than invented ones — a description that omits an argument is recoverable, one that names a parameter that does not exist is not. **A lossy DESCRIBE is costliest exactly when it is most used**, because DESCRIBE is what you reach for once you have stopped trusting the model. Tests `cmd_pages_describe_pageparams_test.go`; the control (explicit-only) reproduces the reported output verbatim. mxcli-formula1 §39 | +| `create or modify odata service` silently revokes the service's access; the next build fails with "At least one allowed role must be selected for the published OData service to be accessible." Re-granting fixes it until the next modify | `serializePublishedODataService` never wrote `AllowedModuleRoles`. The document is serialized wholesale and written with `updateUnit`, so a field the serializer omits is not left alone — it is deleted. The grants were read correctly and carried through the executor, then dropped one layer down | `sdk/mpr/writer_odata.go` (`AllowedModuleRoles`, marker 1 / BY_NAME, matching the working `GRANT` path's `makeMendixStringArray`); stale executor comment corrected in `mdl/executor/cmd_odata.go` | **A wholesale re-serialization deletes every field it does not write, so the writer's field list is a data-retention policy.** Audit it against the parser, not against the struct — the round trip is the contract. **Where you look decides what you conclude**: an earlier pass looked for this loss at model level, found the value present and carried, and recorded "reported but does not reproduce" — the loss only exists after the BSON round trip, so a model-level check could never have seen it. When a report says a value disappears, reproduce at the persistence boundary before disbelieving it. For the marker, copy the shape from the code path that already works (`GRANT` writes marker 1 and builds fine) instead of reasoning from an unrelated type. Tests `writer_odata_test.go`; the control (field omitted again) reproduces the empty-grants document. mxcli-formula1 §26 | +| `.ai-context/skills/` still carries the previous release's guidance after the mxcli binary is upgraded — binary rebuilt at 12:05, skills stamped the day before — with no warning that they disagree | The skills are embedded in the binary and written exactly once, by `mxcli init`. Nothing re-ran init on upgrade, and nothing compared what was on disk against what the binary carried | New `cmd/mxcli/init_skills_sync.go` (`syncAIContextSkills`, `reportSkillSync`), `--sync-skills` flag in `cmd/mxcli/init.go`, and a sync step in the SessionStart bootstrap in `cmd/mxcli/init_hook.go` | **Stale guidance is worse than missing guidance**, because an agent reads it with identical confidence either way — so the failure mode is silent and confident. Fix it where it is consumed, not where it is authored: the SessionStart bootstrap already runs on every session and can fetch the binary, so it is the one place guaranteed to execute immediately before an agent reads the files. Two properties keep an every-session job acceptable: **write only what differs** (an mtime that moves every session makes "when did this last change" unanswerable — a test asserts the mtime holds) and **stay silent when current**. Never fatal: a skills refresh must not block a session, hence `|| true`. A test asserts the bootstrap script actually calls it *before* the exec'd setup, since a step ordered after an `exec` never runs. Tests `init_skills_sync_test.go`; the control (write-once) leaves the stale file in place. mxcli-formula1 §16 | +| `publish entity … (TopSupported: No)` (or `SkipSupported`/`Countable`) parses, `DESCRIBE` reads it back as `No`, and the published `$metadata` still advertises `true`. A client then believes paging works when nothing implements it | `serializeEntitySet` hardcoded all three `QueryOptions` to `true`, ignoring the `*bool` fields the model already carried — the AST/model/DESCRIBE half of the feature shipped without the writer half | `sdk/mpr/writer_odata.go` (`boolOrTrue`, entity-set `QueryOptions`) | **A capability annotation on a microflow-backed resource is load-bearing, not decorative.** Mendix applies *no* query options to a read-microflow resource — it hands the request over and returns what comes back — so the annotation is the only thing a client has to go on, and an over-claim is not cosmetic: the client reads a whole collection believing it is a page. **Check the writer whenever a property round-trips correctly through DESCRIBE**: DESCRIBE reads the model, so a model-only feature describes perfectly and publishes wrong, and the round-trip test everyone reaches for cannot see it. A tri-state `*bool` needs an explicit nil policy at the boundary (`nil` = the platform default, only an explicit `false` opts out) or the pointer is pointless. Tests `writer_odata_test.go`; the control (hardcoded true) reproduces the over-claim. mxcli-formula1 §20 | +| A published OData resource backed by a read microflow silently returns the wrong thing: `?$top=5` yields the whole collection with a 200, and a client re-reading a held row by key gets the collection default and adopts the FIRST row as that object's identity. No error anywhere — well-formed request, valid collection, correct `$count`, 200 | Two promises the service makes on the microflow's behalf and nothing checked: the `KEY` in `expose (…)`, and the `TopSupported`/`SkipSupported` annotations (which default to **true** when unspecified). Mendix applies no query options to a read-microflow resource — it hands over the request and returns what comes back | New `mdl/executor/validate_odata_read_contract.go` (MDL-ODATA02 key promise, MDL-ODATA03 capability over-claim), wired in `cmd/mxcli/cmd_check.go`; the response contract documented in `.claude/skills/mendix/odata-data-sharing.md` | **A read microflow cannot answer 400** — unlike an OData action or an insert/update/delete microflow, the read capability has no `System.HttpResponse` parameter ([docs](https://docs.mendix.com/refguide/published-odata-entity/#custom-http-response)). Its contract is therefore *declarative*: declaring `TopSupported: No` is the read path's only substitute for the refusal it cannot send. **Pick a trigger you can prove**: both rules fire only when the microflow takes no `System.HttpRequest` parameter, because then it provably cannot see a key or a query option; a microflow that does take it gets the benefit of the doubt, since proving *which* options it parses needs real analysis and a rule that guesses gets switched off. **Verify a check rule against a real parse, never a hand-built AST** — the visitor stores `ReadMode` as `MICROFLOW Module.Name` **upper-cased**, so a case-sensitive prefix match made the whole rule dead while it still looked right; the casing is now pinned by its own test. Tests `validate_odata_read_contract_test.go`, examples in `10-odata-examples.mdl` (both correct shapes: request-aware, and honestly declared). mxcli-formula1 §37/§20, suggested issues 2 and 3 | +| No way to see what a running app's subsystem is doing: everything logs at `INFO`, the detail is not in the log at all, and raising the whole runtime to `TRACE` is unusable on a busy app | Nothing in mxcli drove the runtime's per-node log levels, though the M2EE admin API has exposed them all along | New `cmd/mxcli/docker/loglevels.go` (`GetLogSettings`, `SetLogLevels`, `NormalizeLogLevel`, `MatchLogNodes`), `cmd/mxcli/cmd_log.go` (`mxcli log list` / `log set`), documented in `.claude/skills/mendix/analyze-runtime.md` | **Probe an undocumented API before designing the command around it.** Every fact here came from a live 11.12.1 runtime: `get_log_settings` requires one of `node`/`subscriber`/`sort`; `sort` accepts only `node` and `subscriber`; `set_log_level` takes `{"nodes":[{name,level}],"force":bool}`. **The HTTP response for an AdminException says only "See logging output for details" — the actual message ("Please specify node, subscriber or sort option in params", "Unknown sort option: name", "Unknown LogNode X. Use the 'force' parameter…") is in the runtime log**, so probe with the runtime log open or you learn nothing. `force` means "allow a node that does not exist yet" and **permanently registers the name**, so a typo becomes a real empty node — hence it is opt-in, and an unknown node is an error by default. **Resist the subsystem-specific command**: this was proposed as `mxcli odata trace`, but `set_log_level` takes a list of nodes, so the primitive is generic and the OData knowledge belongs in docs. **The node list is a property of the APP, not of Mendix** — a node appears only once something registers it. A first pass concluded "there is no log node for a published OData service" from a project that had none; adding one service turns 57 nodes into 58 and `OData Publish` (with a space) appears, logging the full incoming URI at TRACE — exactly the question that motivated the command. Enumerating capabilities against one sample app and generalising is the trap; `log list` against *this* app is the answer. Distinguish "cannot reach the admin API" from "the runtime refused the request" (`ErrAdminUnreachable`) or the wrong hint buries the real one. Tests `cmd_log_test.go`, `loglevels_test.go`; verified end-to-end against a booted runtime (both arg forms, multi-node, typo refused, `--force` accepted, unreachable port). mxcli-formula1 suggested issue 4 | +| A headless browser cannot log in to an app started with `--hub`, so every screenshot silently shows the login page and rendering defects survive every other check | Under `--hub` the runtime boots with the public **https** root URL, so it marks session cookies `Secure` and prefixes them `__Host-`; a browser on a non-trustworthy http origin cannot store them | `cmd/mxcli/docker/screenshot_login.go` — the login browser context declares `X-Forwarded-Proto: http` when the target is http (Mendix 10.24+ lets that header override `ApplicationRootUrl`) | **The reported cause did not reproduce as stated, and saying so is part of the fix.** On 11.12.1 an https root URL does *not* block a headless browser on `127.0.0.1`: loopback is a **trustworthy origin**, so Chromium accepts `Secure`/`__Host-` cookies there — the app rendered clean, no console errors, no failed requests. The mechanism is real only for a **non-loopback** http origin (a container hostname, a LAN address). Fix shipped anyway because the header is *accurate* rather than a workaround (the request genuinely is http), it costs nothing on an already-http root URL, and real users over https are unaffected. Verified at the layer the bug lives in: the captured Playwright storage state goes from `__Host-XASSESSIONID(secure=true)` to `XASSESSIONID(secure=false)`. **`curl` cannot see this class of bug and neither can a loopback browser** — when a report blames cookie flags, check whether the origin is trustworthy before believing the flags are the blocker. mxcli-formula1 §38 / suggested issue 7 | +| A `--` comment written between two operands of a Mendix expression ends up **inside** the expression; the build fails **CE0117** "Error(s) in expression". `mxcli check` passes and `DESCRIBE` round-trips the comment, so nothing before mxbuild objects | `extractOriginalText` reads the raw input stream between two token positions — which is exactly what preserves an expression's spacing, and also drags in every token the lexer sent to a hidden channel. MDL's `--` and `/* */` are `-> skip`, so they never appear in `ctx.GetText()` but always appear in the source slice | `mdl/visitor/visitor_helpers.go` (`stripMDLComments`, `extractExpressionText`), applied at the six microflow-expression sites in `visitor_microflow_statements.go` / `visitor_microflow_actions.go` | **The ANTLR trap: `ctx.GetText()` excludes hidden tokens, a source-interval slice includes them.** Any code reaching for original text to preserve formatting inherits every comment in that span. **Replace a comment with whitespace, never with nothing** — `1 --c\n+ 2` must not become `1+ 2`, and `'a'--c\n'b'` must not weld into one token. **Respect single-quoted strings**: a Mendix string may legitimately contain `--` or `/*`, and stripping those corrupts the value (tested both, plus the `''` escape that keeps a string open). **Left OQL alone on purpose** — `visitor_entity.go` uses the same helper for view-entity queries, where `--` is legitimate SQL comment syntax; stripping it would change a different language's meaning. **The unit test alone would not have caught a wiring mistake**: it still passed with `extractExpressionText` bypassed, and only the mxbuild run (1 error → 0, same script, same project) proved the call sites were converted. Tests `visitor_strip_comments_test.go`, repro `mdl-examples/bug-tests/comment-in-expression.mdl`. mxcli-formula1 §34 / suggested issue 11 | +| A published OData entity with no `KEY` fails the build with **CE6585** "Published entity 'X' must have a key defined." — so any advice of the form "drop the KEY" is impossible to follow | Mendix requires every published entity to have a key. MDL-ODATA02's suggestion offered "…or drop the KEY" as the alternative to answering a key lookup, and a doctype example demonstrated that non-existent option | `mdl/executor/validate_odata_read_contract.go` (suggestion text), `mdl-examples/doctype-tests/10-odata-examples.mdl`, `.claude/skills/mendix/odata-data-sharing.md` | **Query options you may decline; the key you may not.** A microflow-backed resource whose rows a client can hold *must* answer the key lookup — there is no opt-out, which makes MDL-ODATA02's real remedy singular rather than a choice. **Verify the remedy a diagnostic recommends, not just the diagnosis** — the rule correctly identified an unanswerable KEY and then proposed something mxbuild rejects, which is worse than saying nothing. This is the second time in one session that a doctype example was validated with `mxcli check` (parse-only) instead of the integration gate; `mxcli check` cannot see CE-codes at all, so **any change to `mdl-examples/doctype-tests/` needs `go test -tags integration -run TestMxCheck_DoctypeScripts/