diff --git a/.claude/skills/maintain-wiki.md b/.claude/skills/maintain-wiki.md index 86f092ba7..60fffc093 100644 --- a/.claude/skills/maintain-wiki.md +++ b/.claude/skills/maintain-wiki.md @@ -157,6 +157,9 @@ isn't better served by an existing page or a different doc artifact. | `bug-patterns/styling-compiles-to-nothing.md` | bug-pattern | SCSS and tokens written correctly, compiled nowhere, checked by nothing | | `bug-patterns/package-operations-damage.md` | bug-pattern | marketplace / `mx` operations that change the project and report success | | `bug-patterns/cli-contract-defects.md` | bug-pattern | flags, paths, stdout and help — the class with no Mendix document in it | +| `bug-patterns/engine-divergence.md` | bug-pattern | two backend implementations, and a gap in one that is invisible from inside it | +| `bug-patterns/mutator-addressing.md` | bug-pattern | in-place edits to nodes the model does not name | +| `bug-patterns/access-rule-reconciliation.md` | bug-pattern | GRANT is a read-modify-write, and both directions of loss report success | ## Adding a new page diff --git a/CLAUDE.md b/CLAUDE.md index 6efa2829e..5bb0ca61f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -564,7 +564,7 @@ mxcli uses a layered documentation system — each artifact type has a single ca **ADRs are immutable once accepted.** Supersede with a new ADR rather than editing in place. Conventions and template in [`docs/13-decisions/README.md`](docs/13-decisions/README.md). -**Bug findings are read in the opposite order from how they are written.** A fix *appends* one record to `.claude/skills/fix-issue/findings/.jsonl`; a diagnosis *starts* at `docs-wiki/bug-patterns/`, which digests those records into failure classes, and drills into the findings only for the specific instance. The findings are append-only evidence — grep them, or query them with DuckDB (`select … from 'findings/*.jsonl'`), never read them whole. The digest covers `mdl/executor` (83% of its findings) and `cmd/mxcli` (92%) across 17 pattern pages, and little else; `make digest-status` reports the gap per area, so a pattern miss means "not yet digested", not "not seen before". +**Bug findings are read in the opposite order from how they are written.** A fix *appends* one record to `.claude/skills/fix-issue/findings/.jsonl`; a diagnosis *starts* at `docs-wiki/bug-patterns/`, which digests those records into failure classes, and drills into the findings only for the specific instance. The findings are append-only evidence — grep them, or query them with DuckDB (`select … from 'findings/*.jsonl'`), never read them whole. The digest covers `mdl/executor` (83% of its findings), `cmd/mxcli` (92%) and `mdl/backend` (99%) across 20 pattern pages; `make digest-status` reports the gap per area, so a pattern miss means "not yet digested", not "not seen before". **The wiki is synthesized, not stated.** It frames and connects across the other artifacts — it never restates content that has a canonical home. Rules and seed page list in [`.claude/skills/maintain-wiki.md`](.claude/skills/maintain-wiki.md). diff --git a/docs-wiki/SYNC_LOG.md b/docs-wiki/SYNC_LOG.md index 17f92daef..55d25d60a 100644 --- a/docs-wiki/SYNC_LOG.md +++ b/docs-wiki/SYNC_LOG.md @@ -45,3 +45,7 @@ not capture, because sources are upstream of the commit. | 2026-08-31 | bug-patterns/styling-compiles-to-nothing.md | .claude/skills/fix-issue/findings/cmd-mxcli.jsonl (11 records), cmd/mxcli/theme/block.go | New page (added to seed table). No validator exists for CSS, so location decides whether anything compiles, an unread token is indistinguishable from a design not applied, and contrast is a correctness property nothing measures | | 2026-08-31 | bug-patterns/package-operations-damage.md | .claude/skills/fix-issue/findings/cmd-mxcli.jsonl (11 records), cmd/mxcli/marketplace/update.go | New page (added to seed table). Damage from handing the project to tools mxcli does not control, reported as success — the v2 to v1 collapse at 0 errors being the worst of it | | 2026-08-31 | bug-patterns/cli-contract-defects.md | .claude/skills/fix-issue/findings/cmd-mxcli.jsonl (14 records), cmd/mxcli/syntax/ | New page (added to seed table). The class with no Mendix document in it: help that teaches unparseable syntax, unqualified success messages, accepted-but-inert flags, stdout as a data channel. Matters disproportionately because agents take the tool's word | +| 2026-08-31 | bug-patterns/engine-divergence.md | .claude/skills/fix-issue/findings/mdl-backend.jsonl (30 records), mdl/backend/modelsdk/microflow.go, docs/13-decisions/0004-full-codec-engine.md | New page (added to seed table). First pass over mdl/backend. A gap on one engine is invisible from inside that engine, so the cross-engine DESCRIBE matrix is the only check that finds them; a refusal is honest and a `-- Empty action` placeholder is not; read against the WRITER's keys rather than gen's accessors | +| 2026-08-31 | bug-patterns/mutator-addressing.md | .claude/skills/fix-issue/findings/mdl-backend.jsonl (19 records), mdl/backend/mpr/page_mutator.go, mdl/backend/modelsdk/page_write.go | New page (added to seed table). Many addressable nodes — DataGrid2 columns, object-list items, layout regions — store no Name, so addressing is derived or positional; ambiguity is refused rather than resolved, and hand-built BSON drifts from codec-built BSON in ways only a dump-diff shows | +| 2026-08-31 | bug-patterns/access-rule-reconciliation.md | .claude/skills/fix-issue/findings/mdl-backend.jsonl (6 records), mdl/backend/modelsdk/domainmodel_security_write.go | New page (added to seed table). Small class graded high: GRANT is a read-modify-write and both directions of loss report success. Records that the writer was innocent in the instructive case — a later reconcile removed the rules — and that the reported trigger (WHERE) was a red herring | +| 2026-08-31 | (no page) mdl/backend MCP findings | .claude/skills/fix-issue/findings/mdl-backend.jsonl | Deliberately NOT given a page: ~33 records touch the MCP/PED backend, which already has architecture/mcp-backend.md and models/ped-mutation-constraints.md. Restating them here would violate the never-restate rule; the findings link to those pages instead | diff --git a/docs-wiki/bug-patterns/access-rule-reconciliation.md b/docs-wiki/bug-patterns/access-rule-reconciliation.md new file mode 100644 index 000000000..34181c2bb --- /dev/null +++ b/docs-wiki/bug-patterns/access-rule-reconciliation.md @@ -0,0 +1,62 @@ +--- +title: Access Rules Are Reconciled, Not Appended +category: bug-pattern +last-synced: ced830e0 +sources: + - .claude/skills/fix-issue/findings/mdl-backend.jsonl + - mdl/backend/modelsdk/domainmodel_security_write.go +--- + +> **Do not duplicate**: the GRANT syntax and the security workflow live in +> `.claude/skills/manage-security.md`; the CE numbers and per-member rules live +> in the findings. This page describes why this small class is graded high. + +## What this is + +`GRANT` looks additive and is not. A grant is stored as an *entity access rule* +carrying a complete picture of what a role may do, so writing one is a +read-modify-write over an existing rule — and five `mdl/backend` findings are +that reconciliation losing rights the statement never mentioned. + +The class is small and worth its own page because of what it costs when wrong. +Every other silent loss in this wiki costs a feature; this one **quietly widens +or narrows who can read data**, and both directions are reported as success. + +## How it fits + +**A grant that mentions one attribute must not revoke the others.** The reported +symptom was attributes granted by an earlier statement coming back as `None`. +Structural rights — create, delete, the default member access — went with them, +which the report did not mention: **re-derive the blast radius rather than +inheriting it from the reporter**. + +**The reported trigger is often not the trigger.** A constrained (`WHERE`) grant +was blamed, and the same loss reproduced with no `WHERE` and with `READ *`. A fix +scoped to the reported path would have passed the reporter's reproduction and +left most of the defect in place. + +**When a value is written and then absent, instrument the later pass.** In the +most instructive of these the writer was innocent — it stored all three rules — +and a reconciliation running afterwards removed them. Starting at the writer is +the natural instinct and the wrong end. + +**Preserve what cannot be checked.** Membership questions do not all resolve +locally: an association is qualified by the module that *declares* it, so a +specialization inheriting from a generalization in another module has members the +local walk cannot see. Dropping what the walk cannot confirm produced +`CE0066 "Entity access is out of date"` — the model claiming rights over members +it no longer lists. The safe default is to carry an unconfirmable member through +rather than to prune it. + +**Under-reporting access is the read-side twin.** A restricted page reported as +having "no roles" is the same class seen from the query side, and it is the shape +most likely to be believed, because "no roles" reads like a finding rather than a +gap. + +## See also + +- [fix-issue findings](../../.claude/skills/fix-issue/findings/) — the member + walks, the CE numbers and the controls +- [[association-pointers]] — why a member belongs to the FROM entity, which + decides where a MemberAccess may appear +- [[engine-divergence]] — where the "no roles" read gap came from diff --git a/docs-wiki/bug-patterns/engine-divergence.md b/docs-wiki/bug-patterns/engine-divergence.md new file mode 100644 index 000000000..567adbfa8 --- /dev/null +++ b/docs-wiki/bug-patterns/engine-divergence.md @@ -0,0 +1,77 @@ +--- +title: Two Engines, One Interface +category: bug-pattern +last-synced: ced830e0 +sources: + - .claude/skills/fix-issue/findings/mdl-backend.jsonl + - mdl/backend/modelsdk/microflow.go + - docs/13-decisions/0004-full-codec-engine.md +--- + +> **Do not duplicate**: the abstraction's rationale is canonical in ADR-0002 and +> ADR-0004 and framed in [[backend-abstraction]]; the per-field fixes live in the +> findings. This page describes what goes wrong while two implementations exist. + +## What this is + +The backend interface has two implementations — the legacy `sdk/mpr` writer and +the codec-based `modelsdk` engine — and roughly a quarter of the `mdl/backend` +findings are one of them doing something the other does not. The default is the +newer engine, so a gap in it is the behaviour most users get, while the tests and +habits formed against legacy still pass. + +A gap on one engine is **invisible from inside that engine**. Everything is +self-consistent: the write stores what the read returns, the round trip is +stable, and the field that never existed is never missed. + +## How it fits + +**Two failure modes, and only one of them is honest.** A refusal — +`… not yet supported by the modelsdk engine — rerun with MXCLI_ENGINE=legacy` — +costs the user a flag and tells the truth. The alternative is a placeholder: +`-- Empty action` from a describer that does not recognise an activity type, a +sort clause silently absent, an argument list quietly dropped. The placeholder is +worse than the refusal in every way, because `describe → edit → exec` then +deletes the construct and everything reports success. + +**The worst instance was a read that under-reported access.** A restricted page +came back as "no roles" on the default engine, so `SHOW ACCESS` and the security +matrix both understated who could reach it. A missing field is a cosmetic bug +almost everywhere and a security-relevant one here. + +**The check that finds these is the cross-engine matrix.** Write with engine A, +read with engine B, all four combinations. Every other test shape is +engine-local and therefore blind to exactly this class. + +**Read against the keys the writer builds, not against gen's accessors.** The +generated bindings and the stored document disagree in places — one action's +result is bound as `VariableName` where the model stores `OutputVariableName` — +so an accessor-based reader silently returns nothing. Round-trip tests +(`toGen → encode → decode → fromGen`) catch it; a reader test over hand-written +BSON does not, because the hand-written BSON was written from the same wrong +assumption. These have to be Go tests rather than MDL ones: `create or modify` +rebuilds the document from MDL, so it cannot reproduce a read-back defect at all. + +**Count the gap before fixing the instance.** Decoding every `$Type` through the +real reader turned one reported unsupported action into twenty-seven, one of +which was a shipped feature that could not be described back. Related: several +activity types live in **their own sub-metamodel** — `DatabaseConnector$…`, not +`Microflows$…` — so a probe or a grep under one prefix systematically +under-counts what is covered. + +**Fixing one field and not its sibling is the characteristic mistake.** A reader +restored for `TableMappings` and not `Parameters`, a source type ported without +its key parts. Enumerate every child of the element rather than the one the +report named. + +**Guard the write side with a round trip, not with `mx check`.** Several of these +produced models mxbuild accepts: a box size of zero renders every activity as a +one-pixel sliver in Studio Pro and validates at 0 errors, because `mx check` does +not look at geometry. + +## See also + +- [fix-issue findings](../../.claude/skills/fix-issue/findings/) — the individual + fields, sub-metamodels and round-trip guards +- [[backend-abstraction]] — why the seam exists +- [[describe-round-trip-gaps]] — the same read-side failure, engine-independent diff --git a/docs-wiki/bug-patterns/mutator-addressing.md b/docs-wiki/bug-patterns/mutator-addressing.md new file mode 100644 index 000000000..be6e76517 --- /dev/null +++ b/docs-wiki/bug-patterns/mutator-addressing.md @@ -0,0 +1,74 @@ +--- +title: Addressing Things the Model Does Not Name +category: bug-pattern +last-synced: ced830e0 +sources: + - .claude/skills/fix-issue/findings/mdl-backend.jsonl + - mdl/backend/mpr/page_mutator.go + - mdl/backend/modelsdk/page_write.go +--- + +> **Do not duplicate**: the ALTER PAGE / ALTER LAYOUT syntax lives in the skills +> and `MDL_QUICK_REFERENCE.md`; the per-property fixes live in the findings. This +> page is about the addressing problem underneath them. + +## What this is + +`ALTER PAGE`, `ALTER LAYOUT` and `ALTER WORKFLOW` edit a stored document in +place, which means naming a node inside it. Roughly thirteen `mdl/backend` +findings come from the fact that **many of the interesting nodes have no name**. +A DataGrid 2 column, an Accordion group, a pop-up menu item and a layout region +are all addressable in Studio Pro and store no `Name` at all. + +The resulting failures have a distinctive signature: the command reports +success and nothing changes. Or it reports `widget "X" not found` for something +`DESCRIBE` plainly shows. + +## How it fits + +**A derived name is the only option, and it is not stable.** A column's +addressable name comes from its attribute or caption, so it moves when the +caption is edited. Persisting the name the author wrote is not available either: +there is no slot for it, and inventing one is the write-a-property-Studio-Pro- +does-not-declare hazard. What is left is to make the derivation *visible* — warn +at authoring time which name will be addressable, and on a lookup miss list the +addressable names rather than reporting a bare not-found. + +**Ambiguity must be refused, not resolved.** Two columns deriving the same name +is a real state, and taking the first is a data hazard rather than a wart. The +same applies to a bare name that could mean either an object-list item or a +widget: discriminate on the resolved node's `$Type`, name the qualified form in +the error, and do not guess which grid was meant — guessing is what produced an +invalid document. + +**A slot is not a name.** A layout region has neither, so it is addressed +positionally (`layoutContainer.top`), reusing the dotted reference that also +serves columns and disambiguated the same way. Only `INSERT INTO` accepts one: +BEFORE and AFTER position a widget among siblings, and treating them as INTO +would silently put widgets somewhere the script did not ask for. + +**Property lookup is per-shape, and the shapes differ.** A button's text is a +`CaptionTemplate`, not a `Caption`. Page-level property names are case-sensitive +while widget ones are matched lowercase. A column's value kind comes from the +schema — expression, primitive or text template — and writing a string where a +reference belongs is accepted by everything and visible to nothing. + +**Hand-built BSON drifts from codec-built BSON.** The mutator constructs +documents directly while CREATE goes through the codec, so the two encodings of +"the same" widget diverge — an empty-string value where the codec writes an +explicit key, a typed-array marker present in one and not the other. The reliable +method is to **diff the two encodings** with a BSON dump rather than eyeball the +hand-built one. Getting the marker wrong is not a smaller version of the same +bug: it turns a silent no-op into a project Studio Pro cannot open. + +**Check whether the symptom reproduces outside the reported context.** One +"widget not found" turned out to be two independent defects, and the fix for the +reported nesting alone would not have made the reporter's command work. + +## See also + +- [fix-issue findings](../../.claude/skills/fix-issue/findings/) — the individual + properties, shapes and refusals +- [[unloadable-model-writes]] — where a wrong typed-array marker ends up +- [[silent-property-drop]] — the executor-side twin of "reports success, writes + nothing"