Four formula1 findings: run --local port diagnostics, OData Filter/Sort restrictions, QUAL002 + System-module leak, unbounded external-entity associations - #123
Conversation
`run --local` refused to boot with "a previous 'mxcli run --local' … is
likely still serving on it" and told the user to go hunting with pgrep.
The guess was wrong as often as right, and one of the suggested
patterns — `pgrep -f 'mxcli run'` — matches the shell it is typed into.
The guard now resolves the listener through /proc (inode from
/proc/net/tcp{,6}, owner from /proc/<pid>/fd) and prints its pid and
command line. No lsof/ss: both are routinely missing from slim
containers.
It also separates two cases that had shared one message and need
opposite remedies. A leftover of a previous run — which can only happen
after a kill -9, a crash, or a reaped container, since a graceful stop
already reaps the whole process group — gets a ready-to-paste
`kill <pid>`. A foreign listener gets told it is foreign and pointed at
--app-port, with no kill offered.
Detection-only is unchanged: reaping someone else's process stays the
user's call.
mxcli-formula1 suggested issue 8
FilterRestrictions and SortRestrictions have two shapes and only one was
read. The parser pulled NonFilterableProperties out of the record and
ignored the record's own Bool property, so `Filterable:
!nonFilterable[p.Name]` was true for every property of a set that had
declared nothing filterable — 28 × CE6630 on one service:
'message' is marked Sortable=False in the OData service,
but True in the app.
Mendix picks the shape by arithmetic, not preference: it lists
NonFilterableProperties when SOME attributes are filterable, and emits a
bare `Bool="false" Property="Filterable"` when NONE are, because then
there is no list to write. Both appear in one document, on different
entity sets — an entity exposing only a KEY produces the whole-set form.
Both shapes now sit behind EdmEntitySet.AttrFilterable/AttrSortable, so
a caller cannot consult one and forget the other, which is how this
arose. The accessors are nil-safe, which also removes the entitySet !=
nil dance at the call site. Absent still means true: OData's default is
allowed.
This is the sibling of the Top/Skip fix in 27ea1da — same vocabulary,
same two-shapes cause, found because a consuming app failed to build.
mxcli-formula1 §48
ako
changed the title
A generated project can have documentation nowhere and nothing says so: `mxcli check` and the build both pass, because documentation is never load-bearing. QUAL002 was the reminder, but it reached only entities and microflows. Java actions were not reachable from Starlark at all, and their parameters were not reachable from anywhere — the catalog kept a parameter count and discarded each parameter's Description. That is the field Studio Pro shows to whoever wires up the call, where an undocumented parameter is a blank next to a name like `pInput` at exactly the moment a caller has to decide what to pass. - catalog: java_action_parameters table + view, registered in Tables() - linter: JavaActions() carries its parameters, so a rule naming a parameter can name its action without re-joining; Marketplace and System modules excluded as every sibling iterator does - starlark: java_actions() builtin, parameters nested on each action - QUAL002: Java actions, their parameters, and (off by default, on request) entity attributes — every target switchable via get_option CatalogSchemaVersion 8 -> 9. Without the bump an existing cache keeps its old schema, the query fails, the error is swallowed, and the rule reports zero parameters — silent under-reporting that looks exactly like a documented project.
Extends the previous commit from four targets to all of them. Nineteen document types are now swept by one table-driven projection rather than nineteen bespoke builtins, so covering a new Mendix document type is two rows: one in documentableSources naming the catalog table and its documentation column, one in _DOC_KINDS giving the option and suggestion. On by default, one option each: Module, Entity, Page, Snippet, Building block, Layout, Enumeration, JavaScript action, Image collection, Data transformer, Workflow, Business event service, REST client, Published REST service, Constant, JSON structure, Import mapping, Export mapping. Off by default: attributes and associations — a domain model has hundreds and the same check there is a wall of text rather than a signal. Java action parameters stay on: an action has a handful, and Studio Pro shows each description to whoever wires up the call. Also fixes a leak the sweep made impossible to ignore. modules.Source carries "Marketplace ..." for downloaded modules and is empty for System exactly as it is for the user's own, so the usual `WHERE COALESCE(m.Source,'') = ''` excludes Marketplace and lets all of System through. On a blank 9.24 project that was 52 findings, of which 47 were FileDocument, HttpRequest and friends. Filtering additionally on the sentinel module id leaves the 5 that are the user's. Around ten other LintContext iterators still carry the Source-only filter and leak System into their own rules; those are left alone here rather than silently changing every rule's output in a documentation commit. The documentation column is not uniform — Mendix says Documentation for Java actions, REST and mappings, Description for the rest — and a revert control confirms assuming one spelling silently drops five kinds. Verified end to end, not only in unit tests: exec'd MDL against a real .mpr creating documented and undocumented elements, rebuilt the catalog and ran the CLI, which reports the undocumented ones and stays silent on the documented ones. Unit fixtures insert catalog rows directly and so cannot see a builder that never populates a column.
Follow-up to fbb1609, which fixed the leak only in the iterators it introduced. The same Source-only filter appeared in eleven more places, so every rule that walks entities, pages, microflows, enumerations, constants, snippets, widgets or database connections — plus all three FindUnused kinds — reported platform elements the user cannot change. On a blank Mendix 9.24 project the whole run goes from 60 findings to 8. Removed: CONV001 asking to rename System booleans (User.Active -> IsActive), SEC001 demanding access rules on 38 System entities, DESIGN001 splitting QueuedTask, SEC006 on System.User, MPR003 splitting the System module itself. Verified by diffing full lint output before and after: 52 findings removed, every one of them System, and the ADDED set empty — the predicate only ever narrows, so it cannot invent a finding. TestIterators_ExcludePlatformModules drives all twelve iterators against a catalog holding System, Marketplace and user rows, and asserts each returns the user's element and neither platform one. The third assertion matters most: an iterator returning nothing would satisfy the first two. Also fixes drift in setupModuleFilterDB, whose hand-rolled modules table lacked the Id column the sentinel check reads. Because these iterators swallow query errors and return no rows, that surfaced as four "expected ModA entities to be yielded" failures rather than "no such column".
Same schema drift as setupModuleFilterDB, in two more hand-rolled fixtures. I ran ./mdl/linter/ before committing 4ccf8e5 but not ./mdl/linter/rules/, so four tests in that package were left red. The failure text is misleading in the same way: because the iterators swallow query errors, "no such column: m.Id" reaches the test as "expected 1 violation, got 0".
An iterator whose query failed returned no rows and said nothing:
`if err != nil { return }` inside an iter.Seq[T], which has no error
channel. For a linter that is the worst shape of failure — the entire
output is "here is what I found", so a dead query is indistinguishable
from a clean project, and CI goes green on a run that checked nothing.
This is how three fixtures' missing modules.Id column surfaced as
"expected 1 violation, got 0" rather than "no such column".
LintContext now collects QueryErrors. Iterators still degrade to "no
rows" so one broken query cannot take down the run, but the failure is
recorded, and `mxcli lint` prints each one and exits 1.
All 34 sites: Query+return, Query+continue, bare and inline rows.Scan,
the `return unused` in FindUnused that a bare-return sweep misses, and
the reader-backed ListScheduledEvents. Errors dedupe on iterator+cause,
since several rules iterate the same accessor.
Verified end to end by dropping and recreating a view in a real cached
catalog: the run names the iterator and the cause, exits 1, and the
remedy it prints — delete .mxcli/catalog.db — was run and does clear it.
The healthy path is unchanged: no output, exit 0, same findings.
Part 3 — QUAL002 covers every document type; the System module stops leaking into every ruleFive commits pushed to this branch after the description above was written ( Started from a 3a — the rule now sweeps all 21 document typesJava actions were not reachable from Starlark at all, and their parameters were not reachable from anywhere: the catalog stored a parameter count and discarded each parameter's Rather than nineteen bespoke builtins, one table-driven projection. Adding a Mendix document type is two rows — one in
Attributes and associations are off for volume, not importance: a domain model has hundreds and the same check there is a wall of text. Java action parameters stay on because an action has a handful and each one is read at a call site. Two things the codebase enforced that are easy to miss:
The documentation column is not uniform — Mendix says 3b — System was leaking into every rule, not just this oneWidening the sweep made a much older bug impossible to ignore.
On a blank Mendix 9.24 project, the whole lint run:
Filtering additionally on the sentinel module id ( Verified by diffing the full finding text, not counts: 52 removed, every one a 3c — failed catalog queries are no longer swallowedChasing 3b surfaced the underlying defect. An iterator whose query failed did this: rows, err := ctx.db.Query(...)
if err != nil {
return // iter.Seq[T] has no error channel
}No error, no warning, no log line. For a linter that is the worst available shape of failure: the entire output is "here is what I found", so a dead query is indistinguishable from a clean project and CI goes green on a run that checked nothing. This is exactly how three fixtures' missing
All 34 sites, across five distinct shapes: Signatures stay TestingNineteen new tests. Controls that reproduce, per claim: the generic sweep, each kind list, the doc-column split, Marketplace (both join shapes), System, the no-op recorder (every iterator goes silent), and a single reverted iterator (only that one). Three notes worth recording:
One self-correction: the error message first read ScopeThis is a third concern on a branch that already carried two. By the repo's own checklist ("if the description needs and between unrelated items, split it") these belong in separate PRs; they landed together because the branch was already open. Parts 1–3 touch disjoint files, so they review independently — happy to split 3 out onto its own branch off
🤖 Generated with Claude Code https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 Generated by Claude Code |
ako
changed the title
Part 4 —
|
ako
changed the title
2 participants
Two independent fixes from
ako/mxcli-formula1FINDINGS.md. Unrelated to each other; sectioned so they can be reviewed apart.run --localleaves a child holding the port, and the guard names a process you already killedFilterRestrictions/SortRestrictionshave two annotation shapes and only one was read (28 × CE6630 on one service)Part 1 — suggested issue 8: name the process holding the port
Why only the second remedy
The first one is already implemented.
procgroup_unix.goputs each long-lived child — mxbuild's JVM, the runtime, the rollup bundler — in its own process group, and teardown signals the group, so a graceful stop reaps everything.Which means reaching this error at all tells you something specific: the previous run never ran a handler.
kill -9, a crash, or a reaped container. No in-process change can close that path — by the time the port is stuck, the code that would have freed it was never given a chance to run. So this is a diagnostic fix, deliberately, and not a claim to have stopped the leak.What the guard said, and why it was a guess
"Likely" was doing real work there. An orphan of a previous run and a colleague's unrelated server on 8080 produced identical text and need opposite remedies. And
pgrep -f 'mxcli run'matches the shell you type it into, so following the advice literally can kill your own session.What it says now
The guard resolves the listener through
/proc— inode from/proc/net/tcp{,6}, owner by scanning/proc/<pid>/fdforsocket:[<inode>]. Nolsof/ss: both are routinely absent from slim containers, and needing a subprocess to explain why a boot failed is its own failure mode.Leftover of a previous run — safe to kill, so it offers the command:
Foreign listener — not safe to kill, so it does not offer to:
Both blocks above are real output, captured from the built code against live processes.
Unresolvable (not Linux, or the listener belongs to another user so
/proc/<pid>/fdis unreadable) falls back to a generic hint — minus thepgreppattern that matches the caller's own shell. The guard stays detection-only: reaping someone else's process remains the user's call.Applied to both port guards,
checkTargetPortsFree(run --local) andcheckLocalAppPortsFree(localapp.go).Three details the kernel forces
/proc/net/tcp6, not justtcp[::1]or dual-stack[::]as often as127.0.0.1— the exact process this exists to name. Itslocal_addressis a 128-bit hex blob, not the 32-bit one.0A(TCP_LISTEN)Testing, including two controls that did not fail at first
The strongest test here is also the cheapest: bind a port in the test process and demand
listenerOnPortreturnsos.Getpid(). That cannot pass by accident.Three traps worth recording, because the first two would have shipped as coverage that proved nothing:
tcp6path was effectively untested. This is the same shape as docker build (and therefore docker run / mxcli test) silently converts MPRv2 projects to MPRv1 via mx update-widgets, deleting mprcontents — same root cause as #763, different call site mendixlabs/mxcli#808. Replaced with assertions against captured kernel output.parseListeningInodesdirectly does not provelisteningInodesopens both files. Deleting/proc/net/tcp6from the list left every test green.varso a test can point it at fixtures then stops the test covering the shipped default. Same control, still green. Only after asserting the default's contents did it finally fail:The
TCP_LISTENfilter and the advice wiring have controls that reproduce too. One control turned out to be a no-op rather than a gap —LastIndexvsIndexinhexPort— because a/proclocal_addresshas exactly one colon; that line is defensive, not load-bearing, and is not claimed as tested.Docs
run-local.md(skill + docs-site) previously taught thepgrephunt, including the pattern that matches your own shell. Both now show the real message, explain the leftover-vs-foreign split, and state that a graceful stop already reaps the process group.Part 2 — §48: the sibling of the capabilities bug just fixed
Reported against
715bac5while verifying the §42 fix:27ea1dataught the parser that the OData capabilities vocabulary has two annotation shapes, fixedTopSupported/SkipSupported, and stopped there.FilterRestrictions/SortRestrictionshave the same problem.applyCapabilityAnnotationspulledNonFilterablePropertiesout of the record and ignored the record's ownBoolproperty value.Filterable: !nonFilterable[p.Name]was thentruefor every property of a set that had declared nothing filterable:Same signature as §42: publisher right, contract right, generated consumer wrong.
Mendix picks the shape by arithmetic, not preference
It lists
NonFilterablePropertieswhen some attributes are filterable, and emits a bare boolean when none are — because then there is no list to write. Both appear in one document, on different entity sets:An entity exposing only a KEY reliably produces the second form, which is why a fixture built from the first shape alone tests the half that already worked.
Both shapes behind one accessor
Rather than adding two more fields for the caller to remember to AND together — which is precisely how this arose — the restriction is now one question on the type that owns it:
AttrFilterable/AttrSortableare nil-safe, so theentitySet != nildance and the two lookup maps disappear from the call site. Absent still meanstrue— OData's default is allowed, and defaulting the other way would invert CE6630 for every unannotated service.Both controls reproduce: making the parser ignore the record's
Boolfails the whole-set test with the CE6630 message it describes, and dropping the check insideAttrFilterablefails it the same way. A control test pins the list shape so the half that worked keeps working.Also verified in §48, no change needed
The report confirms five other fixes landed — the OData action (
ededab13509f2a), custom auth (109a55c),AllowedModuleRoles(dc780ec), menu icons (10ba2e19364d43), and the §42 Top/Skip fix itself. Worth surfacing one honest caveat they raise about MDL-ODATA03: their read microflows hand the request to a Java action, so the rule stops at a call it cannot read rather than guessing. That is the designed behaviour — silence over a guess — and it does mean the rule is trusting that app rather than checking it.go test ./mdl/... ./cmd/mxcli/...green;go vetandgofmtclean.🤖 Generated with Claude Code
https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4