diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index 3e132fcc23..3646d3eb8d 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -152,3 +152,5 @@ {"date": "2026-10-03", "area": "cmd/mxcli/check", "symptom": "`mxcli check -p` prints MDL067 (bare commit now WITH events) for a create-or-modify flow already stored with events, which `exec -p` no longer prints", "cause": "cmd_check ran ValidateProgram without the DropSettledCommitNotes filter exec_preflight applies; the project was only connected later, for the reference tier", "fix": "check connects to the project before the semantic report when -p is given, applies DropSettledCommitNotes and StoredTaskClaimViolations, and reuses that connection for the reference tier", "insight": "Two gates over one rule set drift whenever a post-filter lives in only one of them; grep for every caller of ValidateProgram when adding a filter. Control: a flow stored without events still notes", "issue": "ako/mxcli#943", "file": "cmd/mxcli/cmd_check.go", "test": "cmd/mxcli/check_stored_semantics_test.go"} {"date": "2026-10-03", "area": "cmd/mxcli/test", "symptom": "every `mxcli test` run prints 2x MDL-DEPR001 and 2x MDL-V1-SLASH about a script the user never wrote", "cause": "GenerateEndpointMDL emitted a headerless mdl 0 script with `create or replace` and `/` terminators; the test-flow generators had already moved to the version-aware writeScriptHeader/createFlow/writeFlowEnd", "fix": "GenerateEndpointMDL writes mdl 1 through the same helpers (header, create or modify, `;` only); endpoint script is independent of the suite's version", "insight": "A generated script is checked like a user's one; pin it with a test that parses it and asserts ValidateProgram returns nothing. Verified end to end with `mxcli test --local` on a fresh 11.13 app", "issue": "ako/mxcli#943", "file": "cmd/mxcli/testrunner/endpoint.go", "test": "cmd/mxcli/testrunner/endpoint_clean_test.go"} {"date": "2026-10-03", "area": "cmd/mxcli/theme", "symptom": "`theme create acme --from design.css` with `--mxt-font: \"Inter\", system-ui, sans-serif` prints nothing about Inter; the theme ships no woff2 and no @font-face for it and renders in the fallback font wherever Inter is not installed", "cause": "planFonts only decided which VENDORED families to drop; a seeded family outside the vendored set was never looked at, so the silent outcome was the default", "fix": "unvendoredSeededFamilies takes the primary (first) family of each seeded font stack, skips generic families and var() and the families the base partial loads, and CreateResult.UnvendoredFonts carries them to cmd_theme.go, which prints a note per family naming mxcli-fonts/ and the partial", "insight": "Only the first family of a stack is the design's choice; flagging the fallbacks (Helvetica, Arial) would make the note noise. The controls are a vendored family (IBM Plex Mono) and a generic stack, which must stay silent", "issue": "ako/mxcli#944", "file": "cmd/mxcli/theme/create_seeded.go (unvendoredSeededFamilies, planFonts); cmd/mxcli/cmd_theme.go", "test": "cmd/mxcli/theme/create_seeded_test.go (TestCreate_NamesSeededFontsItDoesNotVendor)"} +{"area": "cmd/mxcli/docker", "date": "2026-10-03", "symptom": "`mxcli docker check` (a check) rewrote an MPRv1 project's .mpr permanently, and on v1 and v2 alike rewrote theme-cache/web/theme.compiled.css(.map) and created deployment/sass/main.scss \u2014 with or without --no-update-widgets", "cause": "update-widgets ran on the user's project, protected only by a snapshot/restore of the v2 storage (.mpr + mprcontents/), so v1 had no protection; and `mx check` itself compiles the theme into theme-cache/ and writes deployment/sass/, which nothing guarded", "file": "`cmd/mxcli/docker/check.go` (`Check`), `cmd/mxcli/docker/check_copy.go` (`copyProjectForCheck`)", "insight": "A snapshot of the files you expect a tool to touch protects only those files; measure with a whole-tree hash+mtime diff before/after, which is what showed that plain `mx check` writes too. The fix is to run both mx steps on a temporary copy (skipping deployment/, releases/, theme-cache/, .git, node_modules) and rewrite the copy's path back in mx output. Control: a CE0117 microflow is still reported on both formats with the tree unchanged. `docker build` keeps runUpdateWidgets because it is expected to write deployment/ \u2014 but it still rewrites a v1 .mpr", "refs": ["ako/mxcli#951", "ako/mxcli#568", "ako/mxcli#646"]} +{"area": "cmd/mxcli", "date": "2026-10-03", "symptom": "A project whose CLAUDE.md, skills and lint rules were written by a newer mxcli is served by an older binary (v0.24.0 on PATH) with no warning: 'mdl 1;' is a parse error and shipped lint rules crash, all reading as project defects", "cause": "Nothing recorded which mxcli wrote the tooling; .claude/bootstrap-mxcli.sh linked whatever mxcli was on PATH; init --sync-skills refreshed only .ai-context/skills, never .claude/lint-rules or CLAUDE.md/AGENTS.md", "fix": "init and every sync write .ai-context/mxcli-tooling.json; root PersistentPreRun warns once on stderr when the binary is provably older (release by number, nightly by tag date, mixed by build date, dev never); sync refuses from an older binary; the bootstrap script carries a POSIX-sh copy of the ordering and neither links an older PATH binary nor keeps an older ./mxcli, downloading via a temp file + mv; sync also refreshes bundled lint rules by name and the CLAUDE.md/AGENTS.md section between mxcli:begin/end markers", "insight": "A binary cannot warn about a stamp it predates, so the guard for already-shipped binaries must live in the generated script, which the newer mxcli regenerates. The sh and Go comparisons share one test table so they cannot drift. curl -o ./mxcli on a symlinked ./mxcli would overwrite the PATH binary — always download to a temp name and rename", "issue": "ako/mxcli#952", "file": "cmd/mxcli/tooling_stamp.go; cmd/mxcli/init_tooling_sync.go; cmd/mxcli/init_hook.go (bootstrapScriptTemplate); cmd/mxcli/main.go; cmd/mxcli/init.go", "test": "cmd/mxcli/tooling_stamp_test.go; cmd/mxcli/init_hook_version_test.go; cmd/mxcli/init_tooling_sync_test.go"} diff --git a/.claude/skills/fix-issue/findings/mdl-other.jsonl b/.claude/skills/fix-issue/findings/mdl-other.jsonl index 261f84b4f3..55ee27304d 100644 --- a/.claude/skills/fix-issue/findings/mdl-other.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-other.jsonl @@ -90,3 +90,5 @@ {"area": "mdl/catalog", "date": "2026-10-03", "symptom": "`delete $Order` / `change $Order (…)` inside `loop $Order in $Orders` produce no refs row (no delete/change edge to the entity), while `delete $Orders` on the list does; same for the output of `retrieve $Cust from $Order/Mod.Assoc`. show references / impact under-report batch flows.", "cause": "buildVarEntityMap seeded only object/list parameters and create / database-retrieve outputs, from a flattened action list that had already lost the loop's IterableList; loop iterators and association-retrieve outputs never got an entity, so microflowVarActionRef could not resolve them.", "file": "`mdl/catalog/builder_references.go` (buildVarEntityMap, associationTarget, associationEnds)", "insight": "Walk the object collection, not the flattened actions: the iterator's type lives on the LoopedActivity. Objects is not flow order, so map to a fixpoint (first assignment wins, which also bounds it). An association retrieve's output is the OTHER end from the start variable's entity; when the start is neither end (a specialization) leave it unmapped rather than guess. Control in the test: the list delete that always resolved.", "refs": ["mendixlabs/mxcli#1266"]} {"area": "mdl/linter", "date": "2026-10-03", "symptom": "lint CONV013 reports \"Java action call ... uses '' error handling instead of Custom\" on calls that have `on error { \u2026 }`, and CONV014 never fires on `on error continue` on an action", "cause": "Both rules read BaseActivity.ErrorHandlingType, which the model reader never fills: Mendix stores an action activity's error handling on the ACTION (Microflows$JavaActionCallAction.ErrorHandlingType). The '' in the message was the empty field", "file": "`mdl/linter/rules/conv_error_handling.go`; shared reader `sdk/microflows/error_handling.go` (`ObjectErrorHandlingType`)", "insight": "The unit tests had always set the activity field by hand, so they passed against a shape the reader never produces. Build test objects the way flowObjectFromGen does. Two private reflection helpers (executor DESCRIBE, MCP backend) already read the action correctly; the rules had a third, wrong copy. A '' interpolated into a diagnostic is the cheapest tell of a never-populated field", "refs": ["mendixlabs/mxcli#1202"], "rules": ["CONV013", "CONV014"]} {"area": "mdl/catalog", "date": "2026-10-03", "symptom": "activities table / activities_for() has no rows for anything inside a loop (nested loops included) in a microflow, nanoflow or rule; a Starlark rule cannot find a retrieve, commit or delete in a loop", "cause": "buildMicroflows had three near-copy loops (microflow, nanoflow, rule) over ObjectCollection.Objects that never recursed into LoopedActivity.ObjectCollection, although the reader fills it; countDecisionPoints beside them did recurse", "file": "`mdl/catalog/builder_microflows.go` (`insertFlowActivities`, `countFlowActivities`)", "insight": "Three copies of one walk is how the gap stayed in all three flavours. One shared walker writes ParentLoopId/LoopDepth; activities_for() keeps its top-level default (filtering ParentLoopId = '') so bundled rules such as CONV010 keep their counts, and ActivityCount keeps its top-level meaning beside a new TotalActivityCount. Raw SQL over activities now sees loop-body rows", "refs": ["mendixlabs/mxcli#1266"]} +{"area": "mdl/catalog", "date": "2026-10-03", "symptom": "Parallel mxcli processes on one project (8x `lint` on a fresh copy) print `Warning: failed to save catalog cache: failed to create table catalog_meta: table catalog_meta already exists` or `database is locked (SQLITE_BUSY)`; a reader can open a half-written .mxcli/catalog.db", "cause": "buildCatalog removed the cache and SaveToFile wrote into the path in place: VACUUM INTO refuses a non-empty target another process had just created, and the manual-copy fallback then CREATE TABLEd into that same file. Opening a cache (NewFromFile) also always wrote (createTables + schema_version row), so concurrent openers contended for the write lock", "file": "`mdl/catalog/catalog.go` (`SaveToFile`, `NewFromFile`), `mdl/catalog/catalogdb_sqlite.go`, `mdl/executor/cmd_catalog.go` (buildCatalog save)", "insight": "Write to a temp file in the same directory and os.Rename it over the cache \u2014 but a rename alone moves the failure to readers: SQLite refuses a write on a file renamed out from under an open connection (SQLITE_READONLY_DBMOVED, 'attempt to write a readonly database', 1032). So opening a cache at the current schema version must not write at all; the busy_timeout in the DSN covers the remaining writes of an old-version cache. An in-process test with 8 goroutine writers + 4 reader loops reproduces all three errors deterministically, no subprocesses needed", "refs": ["ako/mxcli#951"]} +{"area": "mdl/linter", "date": "2026-10-03", "symptom": "mxcli report scores a project lower for lint rules that crash: under v0.24.0, project rules written by a newer mxcli (QUAL004, CUSTOM002 reading .document_noun_title) each produced an error-severity 'Starlark rule error: \"microflow\" struct has no .document_noun_title attribute' that counted 10 points against the project", "cause": "StarlarkRule.Check turned every evaluation error into an ordinary SeverityError violation, indistinguishable from a finding; BuildReport and Summarize counted it, and a configured rule severity was applied to it too", "fix": "ruleFailureViolation marks every failure Violation.RuleFailure; a missing struct attribute (matched on the evaluator message, since starlark flattens NoSuchAttrError via fmt.Errorf) becomes info 'rule needs a newer mxcli ()'; BuildReport splits RuleFailures out before counting and every report format lists them separately; Linter.Run skips the severity override for them; an 'undefined:' load failure gets a newer-mxcli hint", "insight": "The score measures the project, so anything about the tooling has to be partitioned out BEFORE counting, not filtered in the formatter. The control that makes the score assertion meaningful is a working rule's finding that does move the score", "issue": "ako/mxcli#952", "file": "mdl/linter/starlark.go (ruleFailureViolation); mdl/linter/report.go (BuildReport); mdl/linter/linter.go (Run); mdl/linter/report_format.go", "test": "mdl/linter/starlark_rule_failure_test.go"} diff --git a/.claude/skills/mendix/custom-widgets/SKILL.md b/.claude/skills/mendix/custom-widgets/SKILL.md index 7826dc847a..5073ff1b55 100644 --- a/.claude/skills/mendix/custom-widgets/SKILL.md +++ b/.claude/skills/mendix/custom-widgets/SKILL.md @@ -303,15 +303,15 @@ MDL032). **CE0463 "update this widget" is EXPECTED after generating charts.** mxcli writes the WidgetType from an embedded 11.6 baseline; the installed Charts.mpk is a -different version, so Studio Pro/mxbuild flags drift. Clear it with **`mxcli docker -check`/`build`** (they normalize the widgets and preserve your storage format). The -whole `mdl-examples/doctype-tests/34-chart-widget-examples.mdl` builds **0 errors** -after normalization. +different version, so Studio Pro/mxbuild flags drift. Clear it with **`mxcli fix +widgets`** (keeps your storage format); `docker check` only normalizes a temp copy, +so check the stored project with `--no-update-widgets`. The whole +`mdl-examples/doctype-tests/34-chart-widget-examples.mdl` builds **0 errors** after. **Do NOT run bare `mx update-widgets` on an MPRv2 project** (an `mprcontents/`-folder project — what `mxcli new` creates): it converts the project to single-file v1 and **deletes `mprcontents/`**, corrupting git, breaking a running `mxcli run --local` -loop, and sometimes making the project unopenable in Studio Pro. `mxcli docker -check`/`build` snapshot/restore the v2 files around the normalization; raw +loop, and sometimes making the project unopenable in Studio Pro. `mxcli fix widgets` +writes the result back as v2, `mxcli docker check` runs on a temporary copy; raw `mx update-widgets` is only safe on a v1 project or a throwaway diagnostic copy. **DESCRIBE round-trips** series/line/scalecolor object-lists (item names are diff --git a/.claude/skills/mendix/migrate-design-prototype/SKILL.md b/.claude/skills/mendix/migrate-design-prototype/SKILL.md index b45d5e165c..f9fc503679 100644 --- a/.claude/skills/mendix/migrate-design-prototype/SKILL.md +++ b/.claude/skills/mendix/migrate-design-prototype/SKILL.md @@ -347,8 +347,8 @@ too — each `series`/`line` binds its own OQL-view datasource + X/Y attributes; Pie/HeatMap bind at the widget level (`ValueAttribute:`, Pie needs `SeriesName:`). See **[Custom & Pluggable Widgets → Charts](../custom-widgets/SKILL.md)** for the chart-type → id table, per-chart required-property gotchas (TimeSeries needs a datetime X, -Bubble needs a size attribute), and the **CE0463 → `mxcli docker check`/`build`** step -(these normalize widgets *and* preserve MPRv2 storage — never run bare +Bubble needs a size attribute), and the **CE0463 → `mxcli fix widgets`** step +(it normalizes the stored widgets *and* preserves MPRv2 storage — never run bare `mx update-widgets` on a `mxcli new` project; it deletes `mprcontents/`). `mdl-examples/doctype-tests/34-chart-widget-examples.mdl` is the full showcase. @@ -659,8 +659,8 @@ the fast index so a design migration doesn't rediscover them. (`Charts.mpk`: column/bar/line/area/pie)** now author via MDL — each `series` (an object-list item inside the chart) binds a datasource plus X/Y attributes: `series s1 (staticDataSource: database from Module.View, staticXAttribute: "X", staticYAttribute: "Y")` - (a per-series OQL view works too). `mxcli docker check`/`build` clear the - widget-version-drift CE0463 (they normalize the widgets and preserve MPRv2 storage — + (a per-series OQL view works too). `mxcli fix widgets` clears the + widget-version-drift CE0463 (it normalizes the stored widgets and preserves MPRv2 storage — do not run bare `mx update-widgets`, which deletes `mprcontents/`). Still lighter when the design allows: a **CSS-background SVG** container (or `HTMLElement`) for sparklines/trends — no datasource — and `ProgressCircle` (`type: expression`, `expressionCurrentValue: '$currentObject/Rate'`, min `'0'` / max `'100'`, diff --git a/CHANGELOG.md b/CHANGELOG.md index f78ba22a38..a0569bc3ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`docker check` no longer modifies the project** (ako/mxcli#951) — `mx update-widgets` and `mx check` now run on a temporary copy, for MPR v1 and v2 alike, and what mx prints names the project's own paths. Before, a check rewrote an MPR v1 project's `.mpr` permanently (only v2 was restored from a snapshot), and `mx check` itself rewrote `theme-cache/` and created `deployment/sass/` even with `--no-update-widgets`. The output now says that widget definitions were normalised on a copy, and that a CE0463 the stored project still has is therefore not reported: `--no-update-widgets` checks the project as stored, `mxcli fix widgets` applies the normalisation (ako/mxcli#568, #646). Build output, caches and VCS folders are not copied; the copy goes to `$TMPDIR` and is removed afterwards. +- **Parallel mxcli runs on one project no longer fail to save the catalog cache** (ako/mxcli#951) — eight parallel `lint` runs on a fresh copy printed `failed to create table catalog_meta: table catalog_meta already exists` or `database is locked`. The cache is now written to a temporary file next to it and renamed into place, so every run saves and a reader sees the old cache or the new one, never a half-written file; opening a current cache no longer writes to it. +- **A lint rule that fails no longer costs the project score** (ako/mxcli#952) — a Starlark rule reading a struct field this mxcli does not expose (a rule written for a newer mxcli, such as one using `document_noun_title` under v0.24.0) is reported at info level as `rule needs a newer mxcli ()` instead of an error. All rule failures are kept out of `mxcli report`'s score, summary and categories and listed in their own "Rules That Could Not Run" section (`ruleFailures` in JSON); other failures stay `Starlark rule error` errors in `mxcli lint`. A configured rule severity no longer applies to the rule's own failure. A rule file that fails to load on an undefined name says the rule may need a newer mxcli. Measured on PedApp with v0.24.0 and rules from main: QUAL004 and CUSTOM002 crashed and scored as 2 errors. - **`create or modify` of a flow matches `commit … with events`, a legacy `split type` spelling and an empty `else` against what is stored** (ako/mxcli#942). Describe prints a stored commit as a bare `commit`, the `when … then` split form, and no empty `else`. The statement diff compared the spelling, so these never matched their own activity. An unchanged re-run reported "Unchanged … (spliced: 1 replaced)". A change inside a loop body next to such a statement was not refused under `mdl 1`, and the loop was rebuilt with new element IDs. `without events` is still a change. **The loop-body refusal also holds when another statement changes next to the loop:** before, only a loop-body change on its own was refused. **`describe` no longer warns that the merge closing an `if` at the end of a loop body "joins no decision"** and would be deleted. The check counted a loop body's flows from the loop's own collection, which holds none. - **Less noise from `check` and `test`** (ako/mxcli#943) — **MDL-WORKFLOW10** no longer warns when the task is claimed in a called microflow: a callee the script creates is read (nested calls too), and with `-p` a stored one; a call that passes the task to a microflow neither can find counts as a possible claim. A callee that does not claim the task it is passed still warns. **`mxcli test`** no longer prints MDL-DEPR001 / MDL-V1-SLASH warnings about the endpoint-registration script it generates itself: that script is `mdl 1`. **`check -p`** drops **MDL067** for a commit already stored the way the script writes it, as `exec` already did. **MDL-WIDGET15** skips a dynamictext with its own `class:` or `style:` (a laid-out label/value pair is not fused text), names the two widgets, and every widget-rule diagnostic now carries its page or snippet as its location instead of "(no module)". - **A required caption with no text in the default language is foreseen** (ako/mxcli#944) — making de_DE the default left a stock app's `Administration.Account_Overview` `tabPage2` (en_US only) empty in de_DE, and mxbuild refused it with CE4899 "Empty caption. [German, Germany]" while `check -p`, `lint` and `exec` said nothing. Measured on 11.14, the tab page caption is the one caption kind the build requires (page titles, buttons, labels, group boxes, column headers, menu items, enumeration captions and messages build without it; page templates and building blocks are not checked). Now: **lint rule QUAL006** (error) lists every tab page caption without the default language; **`alter settings language (DefaultLanguageCode: …)`** prints how many there are and where, with the `alter page … { set (Caption: …) on … }` that fixes one; **`check -p` reports MDL-I18N01** for a script that changes the default — stored captions, and captions the script wrote before the change. And a page created **after** the change in the same script is now written in the new default: the authoring language was resolved once per session, so it was still written in the old one and failed the build too. @@ -119,6 +122,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- **A project records which mxcli wrote its tooling, and an older binary says so** (ako/mxcli#952) — `mxcli init` and every `init --sync-skills` write `.ai-context/mxcli-tooling.json` (version, build time, date; rewritten only when the version changes). Any command that opens the project with `-p` and a binary **older** than the stamp warns once on stderr, naming both versions and how to update; `init --sync-skills` from an older binary **refuses** instead of rolling the skills, rules and CLAUDE.md back. Releases compare by number, nightlies by tag date, a release against a nightly by build date; dev builds are never reported. Binaries from v0.24.0 and earlier cannot read the stamp, so the regenerated `.claude/bootstrap-mxcli.sh` checks it before choosing a binary: an older `mxcli` on PATH is not linked in (it downloads `MXCLI_TAG` instead), an older `./mxcli` is replaced, and the download lands through a temporary file so a `./mxcli` symlink never has it written through into the PATH binary. +- **`init --sync-skills` (alias `--sync`) refreshes the bundled lint rules and the mxcli section of CLAUDE.md / AGENTS.md** (ako/mxcli#952) — it used to refresh only the skills, so a project kept the lint rules and guidance of whichever mxcli first initialised it. Bundled rules are recognised by file name; your own rules beside them are never touched. CLAUDE.md and AGENTS.md are now written between `` / `` markers, and only that section is refreshed — by the sync and by a re-run of `mxcli init` — so project notes outside the markers survive. A file written before the markers is left alone by the sync, with a note; run `mxcli init` once to adopt them. +- **`mxcli init` and `mxcli new` create `mdlsource/`** (ako/mxcli#952) — the directory the generated CLAUDE.md says scripts live in, with a README. - **Starlark lint builtins over the catalog tables that had none** (mendixlabs/mxcli#1265) — `associations()`, `entity_event_handlers()`, `navigation_menu_items()`, `jar_dependencies()`, `strings(language = None)`, `layouts()`, `published_rest_operations()` and `modules()`, each filtered like the other builtins (no System or Marketplace modules, `--modules` / `--exclude`, `--documents` for the document-scoped ones). A rule calling `strings()` gets a full catalog automatically; an untranslated language has no row. Fields are documented in the write-lint-rules skill. - **Association delete behaviour, domain model documentation and the admin user for lint rules** (mendixlabs/mxcli#1269) — `associations()` carries `to_delete_behavior` / `from_delete_behavior` and their error messages (raw Mendix values; CATALOG.ASSOCIATIONS gains the matching columns), `modules()` carries `domain_model_documentation` (CATALOG.MODULES.DomainModelDocumentation; mxcli read it nowhere before, and its write paths keep it), and `project_security()` gains `admin_user_name` and `admin_user_role` — never the password. Catalog schema 17: a cached catalog rebuilds once. - **Activity properties for lint rules** (mendixlabs/mxcli#1266, mendixlabs/mxcli#1267) — `activities_for(name, nested = True)` returns the activities inside loops with `parent_loop_id` and `loop_depth`, and every activity now carries its real `caption` (a split's caption, an annotation's text; it was the placeholder `Activity` on every row), `auto_generate_caption`, `description`, `condition_expression` / `condition_rule` (exclusive splits), `error_handling_type` (`Rollback`, `Custom`, `CustomWithoutRollBack` — capital B — `Continue`, `Abort`), `log_level` / `log_node_expression` / `log_message`, `commit_type` / `with_events`, and `retrieve_source` (`database` / `association`, with `entity_ref` for a database retrieve). A web service call now fills `service_ref`, `action_ref` and its timeout. The same values are columns on the catalog's `activities` table. A stored action mxcli does not model is labelled by its Mendix type (`GenerateJumpToOptionsAction`) instead of `UnsupportedAction`. diff --git a/cmd/mxcli/cmd_new.go b/cmd/mxcli/cmd_new.go index 62b88cf40a..4f967889d6 100644 --- a/cmd/mxcli/cmd_new.go +++ b/cmd/mxcli/cmd_new.go @@ -256,6 +256,11 @@ Examples: } else { fmt.Printf("\nStep 5/7: Skipped (--skip-init)\n") } + // mdlsource/ is where scripts go whatever tooling was chosen — init + // creates it too, this covers --skip-init (ako/mxcli#952). + if _, err := ensureMdlsourceDir(absDir); err != nil { + fmt.Fprintf(os.Stderr, "Warning: creating mdlsource/: %v\n", err) + } // Align the project's Java version with what mxcli can build and run // BEFORE the first build, or that build is the thing that fails: Mendix diff --git a/cmd/mxcli/docker.go b/cmd/mxcli/docker.go index 1c41ee9187..ea2ac40cc9 100644 --- a/cmd/mxcli/docker.go +++ b/cmd/mxcli/docker.go @@ -169,9 +169,18 @@ This catches project errors (broken references, missing attributes, etc.) early, before the slower MxBuild step. The 'docker build' command runs this automatically unless --skip-check is used. -By default, 'mx update-widgets' runs before 'mx check' to normalize -pluggable widget definitions and prevent false CE0463 errors. Use ---no-update-widgets to skip this step. +The check never modifies the project: both mx steps run on a temporary +copy (in $TMPDIR), because mx writes into the project it checks — it +compiles the theme into theme-cache/ and deployment/, and update-widgets +rewrites the model. Build output, caches and VCS folders (deployment/, +releases/, theme-cache/, .git/, node_modules/, ...) are not copied. + +By default, 'mx update-widgets' runs before 'mx check', on that copy, to +normalize pluggable widget definitions and prevent false CE0463 errors. +A CE0463 that normalisation clears is then not reported, although it still +fails MxBuild and 'mxcli run --local' on the stored project. Use +--no-update-widgets to check the project as stored, and 'mxcli fix widgets' +to apply the normalisation to the project. The mx binary is located from the same directory as mxbuild. diff --git a/cmd/mxcli/docker/check.go b/cmd/mxcli/docker/check.go index 91815a0afb..0167520123 100644 --- a/cmd/mxcli/docker/check.go +++ b/cmd/mxcli/docker/check.go @@ -21,8 +21,9 @@ type CheckOptions struct { MxBuildPath string // SkipUpdateWidgets skips the 'mx update-widgets' step before checking. - // By default, update-widgets runs first to normalize pluggable widget - // definitions and prevent false CE0463 errors. + // By default, update-widgets runs first, on the temporary copy the check + // uses, to normalize pluggable widget definitions and prevent false CE0463 + // errors. SkipUpdateWidgets bool // Stdout for output messages. @@ -56,7 +57,24 @@ func copyFile(src, dst string) error { return out.Close() } +// resolveMxForCheck and mxCheckCmd are seams for tests, which substitute stubs +// that behave like the real tools without needing mx. +var resolveMxForCheck = ResolveMxForVersion + +var mxCheckCmd = func(mxPath, mprPath string, w, stderr io.Writer) error { + cmd := exec.Command(mxPath, "check", mprPath) + cmd.Stdout = w + cmd.Stderr = stderr + PrepareMxCommand(cmd) + return cmd.Run() +} + // Check runs 'mx check' on the project to validate it before building. +// +// It never modifies the project. `mx update-widgets` rewrites the model (and +// turns an MPRv2 project into MPRv1), and `mx check` itself writes theme-cache/ +// and deployment/sass/ — so both run on a temporary copy, and what mx prints is +// reported against the project's own paths (ako/mxcli#951). func Check(opts CheckOptions) error { w := opts.Stdout if w == nil { @@ -76,30 +94,56 @@ func Check(opts CheckOptions) error { } } - mxPath, err := ResolveMxForVersion(opts.MxBuildPath, projectVersion) + mxPath, err := resolveMxForCheck(opts.MxBuildPath, projectVersion) if err != nil { return err } fmt.Fprintf(w, "Using mx: %s\n", mxPath) - // Normalize pluggable widget definitions so `mx check` does not report false - // CE0463 ("widget definition changed") errors. runUpdateWidgets preserves the - // project's on-disk storage format; restore is deferred so the check below still - // runs against the widget-normalized model. + projectPath := opts.ProjectPath + if abs, err := filepath.Abs(projectPath); err == nil { + projectPath = abs + } + workMpr, cleanup, err := copyProjectForCheck(projectPath) + if err != nil { + return fmt.Errorf("copy the project to a temporary directory for checking: %w\n"+ + " mx writes into the project it checks, so docker check never runs it on the original;\n"+ + " set TMPDIR to a disk with room for the project", err) + } + defer cleanup() + out := newPathRewriter(w, filepath.Dir(workMpr), filepath.Dir(projectPath)) + errOut := newPathRewriter(stderr, filepath.Dir(workMpr), filepath.Dir(projectPath)) + defer out.Flush() + defer errOut.Flush() + fmt.Fprintln(w, "Checking a temporary copy (mx writes into the project it checks; the project on disk is not changed).") + + // Normalize pluggable widget definitions so `mx check` does not report + // CE0463 ("widget definition changed") for definitions that only need a + // resync. On the copy, so neither the model nor its storage format changes. if !opts.SkipUpdateWidgets { - restore := runUpdateWidgets(mxPath, opts.ProjectPath, w, stderr) - defer restore() + fmt.Fprintln(w, "Normalising widget definitions on the temporary copy...") + if err := updateWidgetsCmd(mxPath, workMpr, out, errOut); err != nil { + out.Flush() + fmt.Fprintf(w, "Warning: update-widgets failed (continuing): %v\n", err) + } } // Run mx check fmt.Fprintf(w, "Checking project %s...\n", opts.ProjectPath) - cmd := exec.Command(mxPath, "check", opts.ProjectPath) - cmd.Stdout = w - cmd.Stderr = stderr - PrepareMxCommand(cmd) + checkErr := mxCheckCmd(mxPath, workMpr, out, errOut) + out.Flush() + errOut.Flush() - if err := cmd.Run(); err != nil { - return fmt.Errorf("project check failed: %w", err) + if !opts.SkipUpdateWidgets { + // #568 / #646: the normalisation can hide a CE0463 the stored project + // still has, which then fails `run --local` and MxBuild. + fmt.Fprintln(w, "Note: widget definitions were normalised on a temporary copy before checking, so a") + fmt.Fprintln(w, " CE0463 the stored project still has is not reported here; it fails MxBuild and") + fmt.Fprintln(w, " `mxcli run --local`. Use --no-update-widgets to check the project as stored, and") + fmt.Fprintln(w, " `mxcli fix widgets` to apply the normalisation to the project.") + } + if checkErr != nil { + return fmt.Errorf("project check failed: %w", checkErr) } fmt.Fprintln(w, "Project check passed.") diff --git a/cmd/mxcli/docker/check_copy.go b/cmd/mxcli/docker/check_copy.go new file mode 100644 index 0000000000..4c9106749a --- /dev/null +++ b/cmd/mxcli/docker/check_copy.go @@ -0,0 +1,167 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "bytes" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "strings" + "sync" +) + +// copyProjectForCheck copies the project that holds mprPath into a fresh +// temporary directory, so `docker check` can run `mx update-widgets` and +// `mx check` there instead of on the user's project (ako/mxcli#951). +// +// Both tools write into the project they are given: update-widgets rewrites the +// model (and converts MPRv2 to MPRv1), and mx check compiles the theme into +// theme-cache/ and writes deployment/sass/. A snapshot/restore of the v2 storage +// covered only the first half of that, and only on v2 — an MPRv1 project's .mpr +// was rewritten permanently by a "check". +// +// Only what mx reads is copied: build output, caches and VCS metadata are skipped +// (copyProjectTree), so the cost is the model plus the widget, theme and source +// folders. cleanup removes the copy; it is never nil and safe to defer. +func copyProjectForCheck(mprPath string) (workMpr string, cleanup func(), err error) { + cleanup = func() {} + abs, err := filepath.Abs(mprPath) + if err != nil { + return "", cleanup, err + } + if info, err := os.Stat(abs); err != nil { + return "", cleanup, err + } else if info.IsDir() { + return "", cleanup, fmt.Errorf("%s is a directory, not a project file", abs) + } + tmp, err := os.MkdirTemp("", "mxcli-check-*") + if err != nil { + return "", cleanup, err + } + cleanup = func() { os.RemoveAll(tmp) } + + projectDir := filepath.Dir(abs) + dst := filepath.Join(tmp, filepath.Base(projectDir)) + if err := copyProjectTree(projectDir, dst); err != nil { + cleanup() + return "", func() {}, err + } + return filepath.Join(dst, filepath.Base(abs)), cleanup, nil +} + +// checkCopySkipRoot are top-level project folders mx check neither needs nor +// should see: build output and caches it regenerates. +var checkCopySkipRoot = map[string]bool{ + "deployment": true, // MxBuild / mx check output + "releases": true, // exported .mda packages + "theme-cache": true, // compiled theme, regenerated by mx check + ".mendix-cache": true, + ".mxcli": true, // catalog cache, widget docs + ".docker": true, // docker init output +} + +// checkCopySkipAnywhere are folder names skipped at any depth. +var checkCopySkipAnywhere = map[string]bool{ + ".git": true, + ".svn": true, + ".hg": true, + "node_modules": true, +} + +// copyProjectTree copies src to dst, skipping the folders above. A symlinked +// file is copied as a file, so a tool writing to the copy cannot write through +// it into the original; a symlinked folder is recreated as a link. +func copyProjectTree(src, dst string) error { + return filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel, err := filepath.Rel(src, p) + if err != nil { + return err + } + target := filepath.Join(dst, rel) + if d.IsDir() { + if p == dst || p == filepath.Dir(dst) { + // $TMPDIR inside the project: do not copy the copy into itself. + return filepath.SkipDir + } + if rel != "." && (checkCopySkipAnywhere[d.Name()] || (!strings.ContainsRune(rel, filepath.Separator) && checkCopySkipRoot[rel])) { + return filepath.SkipDir + } + return os.MkdirAll(target, 0o755) + } + if d.Type()&fs.ModeSymlink != 0 { + info, err := os.Stat(p) + if err != nil { + return nil // dangling link: nothing mx could read either + } + if info.IsDir() { + link, err := os.Readlink(p) + if err != nil { + return err + } + return os.Symlink(link, target) + } + } else if !d.Type().IsRegular() { + return nil // sockets, pipes, devices + } + return copyFile(p, target) + }) +} + +// pathRewriter forwards output line by line with the temporary copy's path +// replaced by the project's own, so what mx reports names the files the user +// has, not a directory that is deleted when the check ends. +type pathRewriter struct { + mu sync.Mutex + w io.Writer + from []string + to string + buf bytes.Buffer +} + +func newPathRewriter(w io.Writer, copyDir, projectDir string) *pathRewriter { + from := []string{copyDir} + if resolved, err := filepath.EvalSymlinks(copyDir); err == nil && resolved != copyDir { + from = append(from, resolved) + } + return &pathRewriter{w: w, from: from, to: projectDir} +} + +func (r *pathRewriter) Write(p []byte) (int, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.buf.Write(p) + for { + i := bytes.IndexByte(r.buf.Bytes(), '\n') + if i < 0 { + break + } + line := string(r.buf.Next(i + 1)) + if _, err := io.WriteString(r.w, r.rewrite(line)); err != nil { + return len(p), err + } + } + return len(p), nil +} + +// Flush writes a trailing partial line. +func (r *pathRewriter) Flush() { + r.mu.Lock() + defer r.mu.Unlock() + if r.buf.Len() > 0 { + io.WriteString(r.w, r.rewrite(r.buf.String())) + r.buf.Reset() + } +} + +func (r *pathRewriter) rewrite(s string) string { + for _, f := range r.from { + s = strings.ReplaceAll(s, f, r.to) + } + return s +} diff --git a/cmd/mxcli/docker/check_integration_test.go b/cmd/mxcli/docker/check_integration_test.go index 981d1244c1..fcfe2bf82e 100644 --- a/cmd/mxcli/docker/check_integration_test.go +++ b/cmd/mxcli/docker/check_integration_test.go @@ -6,6 +6,8 @@ package docker import ( "bytes" + "fmt" + "io" "os" "os/exec" "path/filepath" @@ -79,3 +81,57 @@ func mprStorageVersion(t *testing.T, mprPath string) types.MPRVersion { defer reader.Disconnect() return reader.Version() } + +// TestCheck_LeavesProjectUntouched is the end-to-end guard for ako/mxcli#951: +// with real mx, a default `docker check` and a --no-update-widgets one leave +// every file of an MPRv1 and an MPRv2 project byte-identical with the same +// mtime. Before the fix, update-widgets rewrote a v1 .mpr permanently, and mx +// check wrote theme-cache/ and deployment/sass/ in both formats. +func TestCheck_LeavesProjectUntouched(t *testing.T) { + mxPath, err := ResolveMx("") + if err != nil { + t.Skipf("mx not resolvable: %v", err) + } + scaffold := func(t *testing.T) string { + // Not t.TempDir(): the subtest names make it long enough for + // create-project's template extraction to hit PathTooLongException. + dir, err := os.MkdirTemp("", "chk") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(dir) }) + cmd := exec.Command(mxPath, "create-project") + cmd.Dir = dir + PrepareMxCommand(cmd) + if out, err := cmd.CombinedOutput(); err != nil { + t.Skipf("mx create-project failed: %v\n%s", err, out) + } + return filepath.Join(dir, "App.mpr") + } + + for _, format := range []string{"v2", "v1"} { + for _, skip := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/no-update-widgets=%v", format, skip), func(t *testing.T) { + mprPath := scaffold(t) + if format == "v1" { + // update-widgets converts a v2 project to v1 — the fixture we want. + if err := updateWidgetsCmd(mxPath, mprPath, io.Discard, io.Discard); err != nil { + t.Skipf("could not produce a v1 fixture: %v", err) + } + if v := mprStorageVersion(t, mprPath); v != types.MPRVersionV1 { + t.Skipf("fixture is %v, not v1", v) + } + } + dir := filepath.Dir(mprPath) + before := treeState(t, dir) + var stdout bytes.Buffer + if err := Check(CheckOptions{ProjectPath: mprPath, SkipUpdateWidgets: skip, Stdout: &stdout, Stderr: &stdout}); err != nil { + t.Fatalf("Check: %v\n%s", err, stdout.String()) + } + if d := diffStates(before, treeState(t, dir)); len(d) > 0 { + t.Errorf("docker check modified the project:\n %v", d) + } + }) + } + } +} diff --git a/cmd/mxcli/docker/check_readonly_test.go b/cmd/mxcli/docker/check_readonly_test.go new file mode 100644 index 0000000000..edf4a717df --- /dev/null +++ b/cmd/mxcli/docker/check_readonly_test.go @@ -0,0 +1,275 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#951 item 1: `docker check` must never modify the user's project. +// +// `mx update-widgets` rewrites the model (and turns an MPRv2 project into +// MPRv1), and `mx check` itself compiles the theme into theme-cache/ and writes +// deployment/sass/. Measured on an 11.13 project: an MPRv1 project's .mpr was +// rewritten permanently by a plain `docker check`, and theme-cache/ and +// deployment/ were touched on v1 and v2 alike, with or without +// --no-update-widgets. Check now runs both tools on a temporary copy. +// +// The stubs below do to the directory they are given what the real tools do, +// so these tests fail the moment a tool is pointed at the project itself. +package docker + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// treeState records every file's content hash and mtime, and every directory, +// under root. +func treeState(t *testing.T, root string) map[string]string { + t.Helper() + state := map[string]string{} + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel, _ := filepath.Rel(root, p) + info, err := d.Info() + if err != nil { + return err + } + if d.IsDir() { + state[rel+"/"] = "dir" + return nil + } + b, err := os.ReadFile(p) + if err != nil { + return err + } + sum := sha256.Sum256(b) + state[rel] = hex.EncodeToString(sum[:]) + " " + info.ModTime().UTC().Format(time.RFC3339Nano) + return nil + }) + if err != nil { + t.Fatalf("walk %s: %v", root, err) + } + return state +} + +func diffStates(before, after map[string]string) []string { + var out []string + for k, v := range before { + if a, ok := after[k]; !ok { + out = append(out, "removed "+k) + } else if a != v { + out = append(out, "changed "+k) + } + } + for k := range after { + if _, ok := before[k]; !ok { + out = append(out, "added "+k) + } + } + return out +} + +// addProjectDirs gives a fixture the directories mx touches or reads. +func addProjectDirs(t *testing.T, dir string) { + t.Helper() + for _, f := range []string{ + "widgets/Some.Widget.mpk", + "theme-cache/web/theme.compiled.css", + "theme/web/main.scss", + "javasource/app/Action.java", + } { + p := filepath.Join(dir, f) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte("original "+f), 0o644); err != nil { + t.Fatal(err) + } + } +} + +// stubTools replaces mx resolution and both mx invocations with stubs that +// mutate the project they are pointed at the way the real tools do, and record +// the paths they were given. +func stubTools(t *testing.T) (seen *[]string) { + t.Helper() + var paths []string + origResolve, origUW, origCheck := resolveMxForCheck, updateWidgetsCmd, mxCheckCmd + t.Cleanup(func() { resolveMxForCheck, updateWidgetsCmd, mxCheckCmd = origResolve, origUW, origCheck }) + + resolveMxForCheck = func(string, string) (string, error) { return "mx", nil } + updateWidgetsCmd = func(_, mprPath string, w, _ io.Writer) error { + paths = append(paths, "update-widgets "+mprPath) + dir := filepath.Dir(mprPath) + // Rewrites the model (inlining units on v2) and drops mprcontents/. + if err := os.WriteFile(mprPath, []byte("rewritten by update-widgets"), 0o644); err != nil { + return err + } + return os.RemoveAll(filepath.Join(dir, "mprcontents")) + } + mxCheckCmd = func(_, mprPath string, w, _ io.Writer) error { + paths = append(paths, "check "+mprPath) + dir := filepath.Dir(mprPath) + // Compiles the theme and writes the sass entry point. + if err := os.MkdirAll(filepath.Join(dir, "theme-cache/web"), 0o755); err != nil { + return err + } + if err := os.WriteFile(filepath.Join(dir, "theme-cache/web/theme.compiled.css"), []byte("recompiled"), 0o644); err != nil { + return err + } + if err := os.MkdirAll(filepath.Join(dir, "deployment/sass"), 0o755); err != nil { + return err + } + if err := os.WriteFile(filepath.Join(dir, "deployment/sass/main.scss"), []byte("x"), 0o644); err != nil { + return err + } + // mx reports the project it loaded by path in some messages. + fmt.Fprintf(w, "Loading %s\nThe app contains: 0 errors.\n", mprPath) + return nil + } + return &paths +} + +func TestCheck_DoesNotModifyProject(t *testing.T) { + for _, tc := range []struct { + name string + fixture func(*testing.T) string + skipUpdate bool + wantUWOnACopy bool + }{ + {"v1", v1Fixture, false, true}, + {"v2", v2Fixture, false, true}, + {"v1 --no-update-widgets", v1Fixture, true, false}, + {"v2 --no-update-widgets", v2Fixture, true, false}, + } { + t.Run(tc.name, func(t *testing.T) { + mprPath := tc.fixture(t) + projectDir := filepath.Dir(mprPath) + addProjectDirs(t, projectDir) + seen := stubTools(t) + + tmpRoot := t.TempDir() + t.Setenv("TMPDIR", tmpRoot) + + before := treeState(t, projectDir) + var out bytes.Buffer + if err := Check(CheckOptions{ + ProjectPath: mprPath, + SkipUpdateWidgets: tc.skipUpdate, + Stdout: &out, + Stderr: io.Discard, + }); err != nil { + t.Fatalf("Check: %v\n%s", err, out.String()) + } + after := treeState(t, projectDir) + if d := diffStates(before, after); len(d) > 0 { + t.Errorf("docker check modified the project:\n %s", strings.Join(d, "\n ")) + } + + // Every tool ran, and none of them on the project itself. + if len(*seen) == 0 { + t.Fatal("no mx tool was invoked") + } + for _, s := range *seen { + if strings.HasPrefix(strings.Fields(s)[1], projectDir+string(filepath.Separator)) { + t.Errorf("mx was pointed at the project itself: %s", s) + } + } + ranUW := strings.HasPrefix((*seen)[0], "update-widgets ") + if ranUW != tc.wantUWOnACopy { + t.Errorf("update-widgets ran = %v, want %v (%v)", ranUW, tc.wantUWOnACopy, *seen) + } + + // The output names the project, not the copy, and says what happened. + got := out.String() + if strings.Contains(got, tmpRoot) { + t.Errorf("output leaks the temporary copy's path:\n%s", got) + } + if !strings.Contains(got, "Loading "+mprPath) { + t.Errorf("mx output not reported against the original path:\n%s", got) + } + if tc.wantUWOnACopy && !strings.Contains(got, "normalised on a temporary copy") { + t.Errorf("output does not say widgets were normalised on a copy:\n%s", got) + } + + // The copy is cleaned up. + if entries, _ := os.ReadDir(tmpRoot); len(entries) != 0 { + t.Errorf("temporary copy left behind in %s: %v", tmpRoot, entries) + } + }) + } +} + +// TestCopyProjectForCheck_SkipsOutputs pins that build output, VCS metadata and +// caches are not copied: a large project's deployment/ or .git can dwarf the +// model, and mx check needs neither. +func TestCopyProjectForCheck_SkipsOutputs(t *testing.T) { + src := t.TempDir() + for _, f := range []string{ + "App.mpr", "mprcontents/ab/cd/x.mxunit", "widgets/W.mpk", + "themesource/m/web/main.scss", "theme/web/main.scss", "javasource/a/B.java", + "deployment/run/x.jar", ".git/HEAD", "releases/App.mda", "theme-cache/web/t.css", + ".mxcli/catalog.db", "theme/node_modules/pkg/index.js", ".mendix-cache/x", + } { + p := filepath.Join(src, f) + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte(f), 0o644) + } + dst := t.TempDir() + if err := copyProjectTree(src, dst); err != nil { + t.Fatal(err) + } + for _, f := range []string{"App.mpr", "mprcontents/ab/cd/x.mxunit", "widgets/W.mpk", "themesource/m/web/main.scss", "theme/web/main.scss", "javasource/a/B.java"} { + if _, err := os.Stat(filepath.Join(dst, f)); err != nil { + t.Errorf("%s not copied: %v", f, err) + } + } + for _, f := range []string{"deployment", ".git", "releases", "theme-cache", ".mxcli", "theme/node_modules", ".mendix-cache"} { + if _, err := os.Stat(filepath.Join(dst, f)); err == nil { + t.Errorf("%s was copied; it is output, VCS metadata or a cache", f) + } + } +} + +// TestCopyProjectForCheck_TempDirInsideProject: with $TMPDIR inside the project +// the walk meets its own copy; it must not copy the copy into itself. +func TestCopyProjectForCheck_TempDirInsideProject(t *testing.T) { + proj := t.TempDir() + mpr := filepath.Join(proj, "App.mpr") + if err := os.WriteFile(mpr, []byte("model"), 0o644); err != nil { + t.Fatal(err) + } + tmp := filepath.Join(proj, "tmp") + if err := os.MkdirAll(tmp, 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("TMPDIR", tmp) + work, cleanup, err := copyProjectForCheck(mpr) + if err != nil { + t.Fatalf("copyProjectForCheck: %v", err) + } + defer cleanup() + if b, err := os.ReadFile(work); err != nil || string(b) != "model" { + t.Fatalf("copy of the model = %q, %v", b, err) + } + filepath.WalkDir(filepath.Dir(work), func(p string, d fs.DirEntry, err error) error { + if err == nil && strings.HasPrefix(d.Name(), "mxcli-check-") { + t.Errorf("the copy contains a copy of itself: %s", p) + return filepath.SkipDir + } + return nil + }) +} + +func TestCopyProjectForCheck_MissingProject(t *testing.T) { + if _, _, err := copyProjectForCheck(filepath.Join(t.TempDir(), "nope.mpr")); err == nil { + t.Error("want an error for a project file that does not exist") + } +} diff --git a/cmd/mxcli/docker/check_test.go b/cmd/mxcli/docker/check_test.go index 9ce1d2a4aa..b1ee6d7163 100644 --- a/cmd/mxcli/docker/check_test.go +++ b/cmd/mxcli/docker/check_test.go @@ -157,7 +157,7 @@ func TestCheck_UpdateWidgetsBeforeCheck(t *testing.T) { var stdout, stderr bytes.Buffer opts := CheckOptions{ - ProjectPath: "/tmp/fake.mpr", + ProjectPath: fakeProject(t), MxBuildPath: mxDir, Stdout: &stdout, Stderr: &stderr, @@ -195,7 +195,7 @@ func TestCheck_SkipUpdateWidgetsFlag(t *testing.T) { var stdout, stderr bytes.Buffer opts := CheckOptions{ - ProjectPath: "/tmp/fake.mpr", + ProjectPath: fakeProject(t), MxBuildPath: mxDir, SkipUpdateWidgets: true, Stdout: &stdout, @@ -251,6 +251,7 @@ func TestCheck_UpdateWidgetsReceivesAbsolutePath(t *testing.T) { var stdout, stderr bytes.Buffer // Bare filename — the crash trigger. + t.Chdir(filepath.Dir(fakeProject(t))) Check(CheckOptions{ProjectPath: "fake.mpr", MxBuildPath: dir, Stdout: &stdout, Stderr: &stderr}) logBytes, err := os.ReadFile(logFile) @@ -304,3 +305,15 @@ func TestResolveMxForVersion_PrefersExactCachedVersion(t *testing.T) { t.Errorf("expected exact cached mx %s, got %s", expected, result) } } + +// fakeProject creates an (empty) project file in its own directory. Check copies +// the project's directory before running mx (ako/mxcli#951), so the file has to +// exist, and a directory such as /tmp would be copied whole. +func fakeProject(t *testing.T) string { + t.Helper() + p := filepath.Join(t.TempDir(), "fake.mpr") + if err := os.WriteFile(p, nil, 0o644); err != nil { + t.Fatal(err) + } + return p +} diff --git a/cmd/mxcli/docker/update_widgets.go b/cmd/mxcli/docker/update_widgets.go index 189e3f9275..91c6bfc555 100644 --- a/cmd/mxcli/docker/update_widgets.go +++ b/cmd/mxcli/docker/update_widgets.go @@ -62,7 +62,11 @@ var updateWidgetsCmd = func(mxPath, pathArg string, w, stderr io.Writer) error { // // This lives on the operation, not on a call site, because it was previously // implemented in `Check` only — `Build` carried its own bare invocation and kept -// converting projects (mendixlabs/mxcli#763, then #808). +// converting projects (mendixlabs/mxcli#763, then #808). `Check` no longer uses +// it: it runs update-widgets on a temporary copy (copyProjectForCheck), because a +// check must not modify the project at all — this restores only the v2 storage, +// and an MPRv1 project was rewritten permanently (ako/mxcli#951). `Build` is +// expected to write the project's deployment/, and still uses it. func runUpdateWidgets(mxPath, projectPath string, w, stderr io.Writer) (restore func()) { restore = func() {} if projectPath == "" { diff --git a/cmd/mxcli/init.go b/cmd/mxcli/init.go index 4903afb35a..c26a9ecb26 100644 --- a/cmd/mxcli/init.go +++ b/cmd/mxcli/init.go @@ -23,6 +23,7 @@ var ( initListTools bool initContainerRuntime string initSyncSkills bool + initSync bool ) const mendixGitignore = `# Mendix project @@ -140,16 +141,27 @@ Container Runtime: // init is interactive-ish and writes tool configs; this is the part that // must follow a binary upgrade, and the SessionStart bootstrap runs it // unattended on every session (mxcli-formula1 §16). - if initSyncSkills { - res, err := syncAIContextSkills(absDir) + // + // It also refreshes the bundled lint rules and the mxcli section of + // CLAUDE.md / AGENTS.md, and stamps the version that did it — and + // refuses when this binary is older than that stamp (ako/mxcli#952). + if initSyncSkills || initSync { + res, err := syncProjectTooling(absDir) if err != nil { - fmt.Fprintf(os.Stderr, "Error syncing skills: %v\n", err) + fmt.Fprintf(os.Stderr, "Error syncing project tooling: %v\n", err) os.Exit(1) } - reportSkillSync(os.Stdout, res) + reportToolingSync(os.Stdout, os.Stderr, res) return } + // An explicit init from an older binary still runs — the user asked for + // it — but not silently: it rewrites the tooling a newer mxcli wrote. + if s := staleBinaryStamp(absDir); s != nil { + writeStaleBinaryWarning(os.Stderr, *s) + fmt.Fprintln(os.Stderr, " Continuing: 'mxcli init' will rewrite that tooling with this older version.") + } + // Find .mpr file. With none here, look one level down: a solution repo // keeps each app in its own folder, and running `mxcli init` from the // root used to write everything at the root against an invented @@ -300,8 +312,14 @@ Container Runtime: // Generate content content := file.Content(projectName, mprFile) - // Write file - if err := os.WriteFile(filePath, []byte(content), 0644); err != nil { + // Write file. CLAUDE.md goes through the mxcli markers so a + // later sync can refresh it without touching project notes. + if isOwnedDoc(file.Path) { + if _, err := writeOwnedDoc(filePath, content, true); err != nil { + fmt.Fprintf(os.Stderr, " Error writing %s: %v\n", file.Path, err) + continue + } + } else if err := os.WriteFile(filePath, []byte(content), 0644); err != nil { fmt.Fprintf(os.Stderr, " Error writing %s: %v\n", file.Path, err) continue } @@ -444,7 +462,7 @@ Container Runtime: filePath := filepath.Join(absDir, file.Path) content := file.Content(projectName, mprFile) - if err := os.WriteFile(filePath, []byte(content), 0644); err != nil { + if _, err := writeOwnedDoc(filePath, content, true); err != nil { fmt.Fprintf(os.Stderr, " Error writing %s: %v\n", file.Path, err) os.Exit(1) } @@ -542,6 +560,20 @@ Container Runtime: } } + // The directory the generated CLAUDE.md tells the agent to put scripts + // in; it used to be named and never created (ako/mxcli#952). + if created, err := ensureMdlsourceDir(absDir); err != nil { + fmt.Fprintf(os.Stderr, " Warning: creating mdlsource/: %v\n", err) + } else if created { + fmt.Println("\nCreated mdlsource/ (MDL scripts)") + } + + // Record which mxcli wrote all of the above, so an older binary + // opening the project later can say so (ako/mxcli#952). + if _, err := writeToolingStamp(absDir); err != nil { + fmt.Fprintf(os.Stderr, " Warning: writing %s: %v\n", toolingStampRel, err) + } + fmt.Println("\n✓ Initialization complete!") fmt.Println("\nWhat was created:") fmt.Println(" • .gitignore - Mendix project ignore patterns") @@ -649,7 +681,8 @@ func init() { initCmd.Flags().BoolVar(&initAllTools, "all-tools", false, "Initialize for all supported AI tools") initCmd.Flags().BoolVar(&initListTools, "list-tools", false, "List supported AI tools and exit") initCmd.Flags().StringVar(&initContainerRuntime, "container-runtime", "docker", "Container runtime for devcontainer (docker or podman)") - initCmd.Flags().BoolVar(&initSyncSkills, "sync-skills", false, "Refresh .ai-context/skills/ from this binary and exit (quiet when already current)") + initCmd.Flags().BoolVar(&initSyncSkills, "sync-skills", false, "Refresh the project's mxcli tooling from this binary and exit: skills, bundled lint rules, the mxcli section of CLAUDE.md/AGENTS.md, and the version stamp (quiet when already current; refuses when this binary is older than the stamp)") + initCmd.Flags().BoolVar(&initSync, "sync", false, "Same as --sync-skills") } // findMprFilesInSubdirs returns the .mpr files one level below dir, sorted, so diff --git a/cmd/mxcli/init_hook.go b/cmd/mxcli/init_hook.go index 3c75b84c02..c2b6a9126c 100644 --- a/cmd/mxcli/init_hook.go +++ b/cmd/mxcli/init_hook.go @@ -44,33 +44,121 @@ const bootstrapScriptTemplate = `#!/bin/sh # skipping when it is absent. # # Pin a specific mxcli with MXCLI_TAG=vX.Y.Z (default: nightly). +# +# Version guard: .ai-context/mxcli-tooling.json records the mxcli that wrote +# this project's tooling (CLAUDE.md, skills, lint rules). A binary older than +# that does not understand all of it, so an older ./mxcli is replaced and an +# older mxcli on PATH is not linked in. Only a provable "older" counts: dev +# builds and unknown versions are never second-guessed. Binaries that predate +# the stamp cannot check it themselves — this script is where the check lives +# for them. set -e MPR='%s' TAG="${MXCLI_TAG:-nightly}" +STAMP=.ai-context/mxcli-tooling.json + +# stamp_field KEY: a string field of the tooling stamp, or nothing. +stamp_field() { + [ -f "$STAMP" ] || return 0 + sed -n "s/.*\"$1\": *\"\([^\"]*\)\".*/\1/p" "$STAMP" | head -n 1 +} + +# ymd TIMESTAMP: 2026-10-01T12:00:00Z -> 20261001, anything else -> nothing. +ymd() { + echo "$1" | sed -n 's/^\([0-9]\{4\}\)-\([0-9][0-9]\)-\([0-9][0-9]\)T.*/\1\2\3/p' +} + +# bin_version BINARY / bin_built BINARY: what 'BINARY --version' reports. +bin_version() { + MXCLI_QUIET=1 "$1" --version 2>/dev/null | sed -n 's/^mxcli version \([^ ]*\).*/\1/p' | head -n 1 +} +bin_built() { + ymd "$(MXCLI_QUIET=1 "$1" --version 2>/dev/null | sed -n 's/^mxcli version [^ ]* (\(.*\))$/\1/p' | head -n 1)" +} + +# vclass VERSION: "r MAJOR MINOR PATCH" for a release tag, "n YYYYMMDD" for a +# nightly tag, "u" for anything else (dev builds: v0.24.0-888-g4ba1495f2). +vclass() { + case "$1" in + nightly-[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]-*) + echo "n $(echo "$1" | cut -d- -f2)" ;; + *) + r=$(echo "$1" | sed -n 's/^v\{0,1\}\([0-9][0-9]*\)\.\([0-9][0-9]*\)\.\([0-9][0-9]*\)$/r \1 \2 \3/p') + if [ -n "$r" ]; then echo "$r"; else echo u; fi ;; + esac +} -if [ ! -x ./mxcli ]; then +# version_lt HAVE HAVE_BUILT WANT WANT_BUILT: succeeds when HAVE is provably +# older than WANT. Releases compare by number, nightlies by tag date, and a +# release against a nightly by build date (YYYYMMDD), when both are known. +version_lt() { + hc=$(vclass "$1"); wc=$(vclass "$3"); hb=$2; wb=$4 + [ "$hc" = u ] && return 1 + [ "$wc" = u ] && return 1 + hk=$(echo "$hc" | cut -d' ' -f1); wk=$(echo "$wc" | cut -d' ' -f1) + if [ "$hk" = r ] && [ "$wk" = r ]; then + set -- $(echo "$hc" | cut -d' ' -f2-) $(echo "$wc" | cut -d' ' -f2-) + [ "$1" -lt "$4" ] && return 0 + [ "$1" -gt "$4" ] && return 1 + [ "$2" -lt "$5" ] && return 0 + [ "$2" -gt "$5" ] && return 1 + [ "$3" -lt "$6" ] + return + fi + [ "$hk" = n ] && hb=$(echo "$hc" | cut -d' ' -f2) + [ "$wk" = n ] && wb=$(echo "$wc" | cut -d' ' -f2) + [ -n "$hb" ] && [ -n "$wb" ] && [ "$hb" -lt "$wb" ] +} + +# older_than_stamp BINARY: succeeds when BINARY is provably older than the +# mxcli that wrote this project's tooling. +older_than_stamp() { + want=$(stamp_field version) + [ -n "$want" ] || return 1 + have=$(bin_version "$1") + [ -n "$have" ] || return 1 + version_lt "$have" "$(bin_built "$1")" "$want" "$(ymd "$(stamp_field built)")" +} + +stale= +if [ -x ./mxcli ] && older_than_stamp ./mxcli; then + echo "./mxcli is $(bin_version ./mxcli), older than the mxcli that wrote this project's tooling ($(stamp_field version)) — replacing it." >&2 + stale=1 +fi + +if [ ! -x ./mxcli ] || [ -n "$stale" ]; then # Prefer a copy that is already on this machine. Some environments ship mxcli # pre-installed on PATH, and the bootstrap instructions have you delete the # hardlink 'mxcli new' left in the project — after which this guard could # never be satisfied by the PATH binary and re-downloaded ~85 MB on EVERY # fresh session, forever. (ako/ChipCoV1) # + # But never one older than the tooling: that is how a project written by a + # newer mxcli ended up served by v0.24.0, its CLAUDE.md asking for syntax the + # binary could not parse (ako/mxcli#952). + # # Hardlink first because that is what 'mxcli new' does and it costs nothing; # fall back to a symlink across filesystems, then to a copy. Any of the three # leaves ./mxcli working, which is what the rest of this script and the # project's own CLAUDE.md assume. onpath=$(command -v mxcli 2>/dev/null || true) - if [ -n "$onpath" ] && [ -x "$onpath" ]; then - echo "mxcli found on PATH (${onpath}) — linking it in rather than downloading." - ln -f "$onpath" ./mxcli 2>/dev/null || - ln -sf "$onpath" ./mxcli 2>/dev/null || - cp "$onpath" ./mxcli - chmod +x ./mxcli 2>/dev/null || true + if [ -n "$onpath" ] && [ -x "$onpath" ] && [ "$onpath" != "./mxcli" ] && [ "$onpath" != "$PWD/mxcli" ]; then + if older_than_stamp "$onpath"; then + echo "mxcli on PATH (${onpath}) is $(bin_version "$onpath"), older than this project's tooling ($(stamp_field version)) — not linking it; downloading ${TAG} instead." >&2 + else + echo "mxcli found on PATH (${onpath}) — linking it in rather than downloading." + rm -f ./mxcli + ln -f "$onpath" ./mxcli 2>/dev/null || + ln -sf "$onpath" ./mxcli 2>/dev/null || + cp "$onpath" ./mxcli + chmod +x ./mxcli 2>/dev/null || true + stale= + fi fi fi -if [ ! -x ./mxcli ]; then +if [ ! -x ./mxcli ] || [ -n "$stale" ]; then os=$(uname -s | tr 'A-Z' 'a-z') case "$(uname -m)" in x86_64|amd64) arch=amd64 ;; @@ -78,20 +166,36 @@ if [ ! -x ./mxcli ]; then *) arch=$(uname -m) ;; esac url="https://github.com/mendixlabs/mxcli/releases/download/${TAG}/mxcli-${os}-${arch}" - echo "mxcli not found — downloading ${TAG} for ${os}/${arch}..." - if ! curl -fsSL -o ./mxcli "$url"; then + echo "Downloading mxcli ${TAG} for ${os}/${arch}..." + # Into a temporary file, then renamed over ./mxcli: writing straight to + # ./mxcli would write THROUGH a symlink into the PATH binary it points at, + # and a failed download would leave no binary at all. + if curl -fsSL -o ./mxcli.download "$url"; then + chmod +x ./mxcli.download + mv -f ./mxcli.download ./mxcli + else + rm -f ./mxcli.download echo "Could not download mxcli from ${url}." >&2 - echo "Fetch it manually, or set MXCLI_TAG to a released version." >&2 - exit 1 + if [ ! -x ./mxcli ]; then + echo "Fetch it manually, or set MXCLI_TAG to a released version." >&2 + exit 1 + fi + echo "Keeping the older ./mxcli; expect failures where the tooling uses newer features." >&2 fi - chmod +x ./mxcli fi -# Keep .ai-context/skills/ in step with this binary. The skills are embedded in -# mxcli and written once by 'mxcli init', so upgrading the binary used to leave -# yesterday's guidance in place with no warning — and an agent reads stale -# guidance with the same confidence as current guidance. Quiet when already -# current; never fatal, since a skills refresh must not block the session. +if older_than_stamp ./mxcli; then + echo "WARNING: ./mxcli ($(bin_version ./mxcli)) is still older than this project's tooling ($(stamp_field version))." >&2 + echo " Set MXCLI_TAG to $(stamp_field version) or newer (or 'nightly') and re-run this script." >&2 +fi + +# Keep the project's tooling (skills, bundled lint rules, the mxcli section of +# CLAUDE.md/AGENTS.md) in step with this binary. They are embedded in mxcli and +# written by 'mxcli init', so upgrading the binary used to leave yesterday's +# guidance in place with no warning — and an agent reads stale guidance with +# the same confidence as current guidance. Quiet when already current; refuses +# (rather than downgrades) when the binary is older than the tooling; never +# fatal, since a refresh must not block the session. ./mxcli init --sync-skills . || true exec ./mxcli run --local --setup --ensure-db -p "$MPR" diff --git a/cmd/mxcli/init_hook_version_test.go b/cmd/mxcli/init_hook_version_test.go new file mode 100644 index 0000000000..4e6a86a206 --- /dev/null +++ b/cmd/mxcli/init_hook_version_test.go @@ -0,0 +1,219 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// The bootstrap script decides which mxcli serves the project before any +// mxcli runs, so it carries its own copy of the version ordering. These tests +// run the generated script under sh with stand-in binaries (ako/mxcli#952). + +func requireSh(t *testing.T) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("the bootstrap script is POSIX sh") + } + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("no sh") + } +} + +// bootstrapFunctions is the script's helper-function prelude: everything +// before the first statement that acts. +func bootstrapFunctions(t *testing.T) string { + t.Helper() + script := fmt.Sprintf(bootstrapScriptTemplate, "App.mpr") + i := strings.Index(script, "\nstale=\n") + if i < 0 { + t.Fatal("bootstrap script has no 'stale=' line to cut the prelude at") + } + return strings.Replace(script[:i], "set -e\n", "", 1) +} + +// TestBootstrapVersionGuard_AgreesWithGo runs the script's version_lt over the +// same table the Go comparison is tested with. +func TestBootstrapVersionGuard_AgreesWithGo(t *testing.T) { + requireSh(t) + prelude := bootstrapFunctions(t) + for _, c := range versionOrderCases { + t.Run(c.name, func(t *testing.T) { + sh := prelude + fmt.Sprintf("\nif version_lt %q \"$(ymd %q)\" %q \"$(ymd %q)\"; then echo older; else echo not; fi\n", + c.have, c.haveBuilt, c.want, c.wantBuilt) + out, err := exec.Command("sh", "-c", sh).CombinedOutput() + if err != nil { + t.Fatalf("sh: %v\n%s", err, out) + } + got := strings.TrimSpace(string(out)) == "older" + if got != c.older { + t.Errorf("sh version_lt(%s, %s) = %v (output %q), want %v", c.have, c.want, got, out, c.older) + } + }) + } +} + +// fakeMxcli writes a stand-in mxcli reporting version, logging every other +// invocation so a test can tell which binary ran the rest of the script. +func fakeMxcli(t *testing.T, path, version string) { + t.Helper() + body := "#!/bin/sh\n" + + "if [ \"$1\" = \"--version\" ]; then echo \"mxcli version " + version + " (2026-10-01T00:00:00Z)\"; exit 0; fi\n" + + "echo \"" + version + " $*\" >> \"$BOOT_LOG\"\n" + if err := os.WriteFile(path, []byte(body), 0o755); err != nil { + t.Fatal(err) + } +} + +type bootEnv struct { + project, bin, log string +} + +// newBootEnv sets up a project with the bootstrap script, an optional stamp, +// and a bin dir holding a fake curl that "downloads" a nightly from the future. +func newBootEnv(t *testing.T, stampVersion string) bootEnv { + t.Helper() + root := t.TempDir() + e := bootEnv{project: filepath.Join(root, "app"), bin: filepath.Join(root, "bin"), log: filepath.Join(root, "boot.log")} + for _, d := range []string{e.project, e.bin} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + if _, err := writeBootstrapScript(filepath.Join(e.project, ".claude"), "App.mpr"); err != nil { + t.Fatal(err) + } + if stampVersion != "" { + stamp := fmt.Sprintf("{\n \"version\": %q,\n \"written\": \"2026-10-01\"\n}\n", stampVersion) + if err := os.MkdirAll(filepath.Join(e.project, ".ai-context"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(e.project, toolingStampRel), []byte(stamp), 0o644); err != nil { + t.Fatal(err) + } + } + // curl -fsSL -o FILE URL: write a nightly stand-in to FILE. + curl := "#!/bin/sh\nout=\nwhile [ $# -gt 0 ]; do if [ \"$1\" = \"-o\" ]; then out=$2; shift; fi; shift; done\n" + + "echo \"curl $out\" >> \"$BOOT_LOG\"\n" + + "printf '#!/bin/sh\\nif [ \"$1\" = \"--version\" ]; then echo \"mxcli version nightly-20991231-abcdef0\"; exit 0; fi\\necho \"nightly $*\" >> \"$BOOT_LOG\"\\n' > \"$out\"\n" + if err := os.WriteFile(filepath.Join(e.bin, "curl"), []byte(curl), 0o755); err != nil { + t.Fatal(err) + } + return e +} + +func (e bootEnv) run(t *testing.T) (stderr string) { + t.Helper() + cmd := exec.Command("sh", bootstrapScriptName) + cmd.Dir = e.project + cmd.Env = append(os.Environ(), "PATH="+e.bin+":/usr/bin:/bin", "BOOT_LOG="+e.log, "MXCLI_TAG=") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("bootstrap failed: %v\n%s", err, out) + } + return string(out) +} + +func (e bootEnv) projectBinaryVersion(t *testing.T) string { + t.Helper() + out, err := exec.Command(filepath.Join(e.project, "mxcli"), "--version").Output() + if err != nil { + t.Fatalf("./mxcli --version: %v", err) + } + return strings.Fields(string(out))[2] +} + +func (e bootEnv) downloaded(t *testing.T) bool { + data, _ := os.ReadFile(e.log) + return strings.Contains(string(data), "curl ") +} + +// The reported skew: v0.24.0 on PATH, tooling from a newer mxcli. The old +// bootstrap linked it in; the guard refuses and downloads instead. +func TestBootstrapVersionGuard_OlderPathBinaryIsNotLinked(t *testing.T) { + requireSh(t) + e := newBootEnv(t, "v0.25.0") + fakeMxcli(t, filepath.Join(e.bin, "mxcli"), "v0.24.0") + + out := e.run(t) + if !e.downloaded(t) { + t.Errorf("older PATH binary was linked instead of downloading:\n%s", out) + } + if got := e.projectBinaryVersion(t); got != "nightly-20991231-abcdef0" { + t.Errorf("./mxcli is %s, want the downloaded nightly", got) + } + if !strings.Contains(out, "not linking it") { + t.Errorf("no explanation for skipping the PATH binary:\n%s", out) + } +} + +// Control: a PATH binary at least as new as the stamp is linked, no download. +func TestBootstrapVersionGuard_NewerPathBinaryIsLinked(t *testing.T) { + requireSh(t) + for _, c := range []struct{ name, stamp, onPath string }{ + {"newer than stamp", "v0.24.0", "v0.25.0"}, + {"no stamp (pre-#952 project)", "", "v0.24.0"}, + {"dev build is never second-guessed", "v0.25.0", "v0.24.0-888-g4ba1495f2"}, + } { + t.Run(c.name, func(t *testing.T) { + e := newBootEnv(t, c.stamp) + fakeMxcli(t, filepath.Join(e.bin, "mxcli"), c.onPath) + out := e.run(t) + if e.downloaded(t) { + t.Errorf("downloaded although the PATH binary qualifies:\n%s", out) + } + if got := e.projectBinaryVersion(t); got != c.onPath { + t.Errorf("./mxcli is %s, want the PATH binary %s", got, c.onPath) + } + }) + } +} + +// A project binary older than the stamp is replaced — by a newer PATH binary +// when there is one. +func TestBootstrapVersionGuard_StaleProjectBinaryIsReplaced(t *testing.T) { + requireSh(t) + e := newBootEnv(t, "v0.25.0") + fakeMxcli(t, filepath.Join(e.project, "mxcli"), "v0.24.0") + fakeMxcli(t, filepath.Join(e.bin, "mxcli"), "v0.26.0") + + out := e.run(t) + if got := e.projectBinaryVersion(t); got != "v0.26.0" { + t.Errorf("./mxcli is %s, want v0.26.0 from PATH:\n%s", got, out) + } + if e.downloaded(t) { + t.Error("downloaded although PATH had a newer binary") + } + // The rest of the script ran on the replacement, not the stale binary. + log, _ := os.ReadFile(e.log) + if strings.Contains(string(log), "v0.24.0 ") { + t.Errorf("stale binary still ran:\n%s", log) + } +} + +// ./mxcli symlinked to an old PATH binary: the replacement must not be +// written through the link into the PATH binary itself. +func TestBootstrapVersionGuard_DownloadDoesNotWriteThroughSymlink(t *testing.T) { + requireSh(t) + e := newBootEnv(t, "v0.25.0") + onPath := filepath.Join(e.bin, "mxcli") + fakeMxcli(t, onPath, "v0.24.0") + if err := os.Symlink(onPath, filepath.Join(e.project, "mxcli")); err != nil { + t.Fatal(err) + } + + e.run(t) + if got := e.projectBinaryVersion(t); got != "nightly-20991231-abcdef0" { + t.Errorf("./mxcli is %s, want the downloaded nightly", got) + } + out, err := exec.Command(onPath, "--version").Output() + if err != nil || !strings.Contains(string(out), "v0.24.0") { + t.Errorf("PATH binary was modified: %q %v", out, err) + } +} diff --git a/cmd/mxcli/init_tooling_sync.go b/cmd/mxcli/init_tooling_sync.go new file mode 100644 index 0000000000..802da90852 --- /dev/null +++ b/cmd/mxcli/init_tooling_sync.go @@ -0,0 +1,323 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" +) + +// init_tooling_sync.go extends the session-start refresh beyond the skills to +// the rest of what `mxcli init` writes from the binary: the bundled lint rules +// and the mxcli-owned part of CLAUDE.md / AGENTS.md (ako/mxcli#952). +// +// Before this, `init --sync-skills` refreshed only the skills. A project kept +// the lint rules and CLAUDE.md of whichever mxcli first initialised it, so +// after an upgrade the guidance and the rules disagreed with the binary in +// exactly the way the skills sync was written to prevent. + +// Markers delimiting the part of a generated markdown file mxcli owns. Matched +// by prefix, so the wording of the begin line can change without orphaning +// the files that carry an older one. +const ( + mxcliSectionBeginPrefix = "" + mxcliSectionEnd = "" +) + +// ownedDocs are the generated markdown files whose mxcli section is kept +// current. Each is refreshed only when it already exists: the sync never +// creates a file `mxcli init` did not (a Cursor-only project has no CLAUDE.md, +// and must not grow one). +func ownedDocs() []ToolFile { + var docs []ToolFile + for _, f := range SupportedTools["claude"].Files { + if f.Path == "CLAUDE.md" { + docs = append(docs, f) + } + } + return append(docs, UniversalFiles...) +} + +// isOwnedDoc reports whether init should write path through the markers. +func isOwnedDoc(path string) bool { + for _, d := range ownedDocs() { + if d.Path == path { + return true + } + } + return false +} + +// wrapMxcliSection puts generated content between the markers. +func wrapMxcliSection(generated string) string { + if !strings.HasSuffix(generated, "\n") { + generated += "\n" + } + return mxcliSectionBegin + "\n" + generated + mxcliSectionEnd + "\n" +} + +// mergeMxcliSection replaces the marked section of existing with generated, +// keeping every byte outside the markers. ok is false when existing has no +// well-formed marker pair, in which case nothing can be told apart and the +// caller must not write. +func mergeMxcliSection(existing, generated string) (out string, ok bool) { + begin := lineStartingWith(existing, mxcliSectionBeginPrefix, 0) + if begin < 0 { + return "", false + } + end := lineStartingWith(existing, mxcliSectionEnd, begin) + if end < 0 { + return "", false + } + after := end + len(mxcliSectionEnd) + // Consume the rest of the end-marker line, including its newline. + if nl := strings.IndexByte(existing[after:], '\n'); nl >= 0 { + after += nl + 1 + } else { + after = len(existing) + } + return existing[:begin] + wrapMxcliSection(generated) + existing[after:], true +} + +// lineStartingWith returns the offset of the first line at or after from that +// begins with prefix, or -1. +func lineStartingWith(s, prefix string, from int) int { + for i := from; i < len(s); { + if (i == 0 || s[i-1] == '\n') && strings.HasPrefix(s[i:], prefix) { + return i + } + nl := strings.IndexByte(s[i:], '\n') + if nl < 0 { + break + } + i += nl + 1 + } + return -1 +} + +// docWriteResult is what writing one owned doc did. +type docWriteResult int + +const ( + docUnchanged docWriteResult = iota + docWritten + docNoMarkers // exists without markers; left alone +) + +// writeOwnedDoc writes the generated content of an mxcli-owned markdown file. +// +// A file with markers has only its marked section replaced. A file without +// them predates the markers: `mxcli init` regenerates it whole, as it always +// has (adopting the markers from then on), while the unattended sync leaves it +// untouched — it cannot tell mxcli's text from the project's, and a refresh +// that runs on every session start must never be the thing that deletes a +// project's notes. +func writeOwnedDoc(path, generated string, fromInit bool) (docWriteResult, error) { + existing, err := os.ReadFile(path) + var want string + switch { + case os.IsNotExist(err): + if !fromInit { + return docUnchanged, nil + } + want = wrapMxcliSection(generated) + case err != nil: + return docUnchanged, err + default: + merged, ok := mergeMxcliSection(string(existing), generated) + switch { + case ok: + want = merged + case fromInit: + want = wrapMxcliSection(generated) + default: + return docNoMarkers, nil + } + if want == string(existing) { + return docUnchanged, nil + } + } + if err := os.WriteFile(path, []byte(want), 0o644); err != nil { + return docUnchanged, fmt.Errorf("writing %s: %w", path, err) + } + return docWritten, nil +} + +// bundledLintRuleNames lists the rule files this binary ships, by file name. +// The name IS the marker of a bundled rule: a project's own rules live beside +// them in the same directory under other names, and are never touched. +func bundledLintRuleNames() ([]string, error) { + var names []string + err := fs.WalkDir(lintRulesFS, "lint-rules", func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() { + names = append(names, d.Name()) + } + return nil + }) + sort.Strings(names) + return names, err +} + +// syncBundledLintRules rewrites the bundled rules in a project's lint-rules +// directory from the binary, returning the names that differed. A missing +// directory means the project was not initialised with a tool that uses the +// rules, and stays missing. +// +// An edit to a bundled rule file is overwritten — the file belongs to mxcli, +// like a skill. To change one, copy it under a new name (and a new rule ID), +// or override it in lint-config.yaml. +func syncBundledLintRules(lintRulesDir string) ([]string, error) { + if st, err := os.Stat(lintRulesDir); err != nil || !st.IsDir() { + return nil, nil + } + names, err := bundledLintRuleNames() + if err != nil { + return nil, err + } + var changed []string + for _, name := range names { + want, err := lintRulesFS.ReadFile("lint-rules/" + name) + if err != nil { + return changed, err + } + target := filepath.Join(lintRulesDir, name) + if have, readErr := os.ReadFile(target); readErr == nil && bytes.Equal(have, want) { + continue + } + if err := os.WriteFile(target, want, 0o644); err != nil { + return changed, fmt.Errorf("writing %s: %w", target, err) + } + changed = append(changed, name) + } + return changed, nil +} + +// toolingSyncResult reports a full tooling refresh. +type toolingSyncResult struct { + Skills skillSyncResult + LintRules []string // bundled rule files rewritten + Docs []string // owned docs whose mxcli section was rewritten + NoMarkers []string // owned docs left alone: no markers to refresh between + StampWritten bool + // RefusedFor is set when the sync did nothing because this binary is older + // than the one that wrote the tooling. + RefusedFor *toolingStamp +} + +// syncProjectTooling is `mxcli init --sync-skills`: refresh everything init +// derives from the binary, without touching what the project wrote. +// +// It refuses outright when the running binary is older than the stamp. The +// refresh is a copy from the binary, so running it from an older one is a +// downgrade — the skills, the rules and CLAUDE.md all moved back to what the +// old binary knows, silently, on the next session start. Updating the binary is +// the fix; rolling the project back to match it is not. +func syncProjectTooling(projectDir string) (toolingSyncResult, error) { + var res toolingSyncResult + if s := staleBinaryStamp(projectDir); s != nil { + res.RefusedFor = s + return res, nil + } + + skills, err := syncAIContextSkills(projectDir) + if err != nil { + return res, fmt.Errorf("syncing skills: %w", err) + } + res.Skills = skills + + if res.LintRules, err = syncBundledLintRules(filepath.Join(projectDir, ".claude", "lint-rules")); err != nil { + return res, fmt.Errorf("syncing lint rules: %w", err) + } + + // The docs embed the .mpr path, so without one there is nothing correct to + // render; init would write a placeholder, a refresh must not. + if mprFile := findMprFile(projectDir); mprFile != "" { + projectName := filepath.Base(projectDir) + for _, doc := range ownedDocs() { + r, err := writeOwnedDoc(filepath.Join(projectDir, doc.Path), doc.Content(projectName, mprFile), false) + if err != nil { + return res, err + } + switch r { + case docWritten: + res.Docs = append(res.Docs, doc.Path) + case docNoMarkers: + res.NoMarkers = append(res.NoMarkers, doc.Path) + } + } + } + + if res.StampWritten, err = writeToolingStamp(projectDir); err != nil { + return res, err + } + return res, nil +} + +// reportToolingSync prints what the refresh did — nothing at all when the +// project was already current, since this runs on every session start. The +// refusal and the marker notice go to errw: they are about the setup, not the +// refresh, and must be visible even when stdout is discarded. +func reportToolingSync(w, errw io.Writer, res toolingSyncResult) { + if res.RefusedFor != nil { + fmt.Fprintf(errw, "Not syncing: this mxcli (%s) is older than the mxcli that wrote this project's tooling (%s);\n", + currentMxcliVersion(), res.RefusedFor.Version) + fmt.Fprintln(errw, " a sync from it would downgrade the skills, lint rules and CLAUDE.md. Update mxcli instead:") + fmt.Fprintf(errw, " ./mxcli setup mxcli --tag %s --output ./mxcli, or delete ./mxcli and run: sh %s\n", + updateTag(res.RefusedFor.Version), bootstrapScriptName) + return + } + reportSkillSync(w, res.Skills) + if len(res.LintRules) > 0 { + fmt.Fprintf(w, "Refreshed %d bundled lint rule(s) in .claude/lint-rules/ to match this mxcli: %s\n", + len(res.LintRules), abridge(res.LintRules)) + } + if len(res.Docs) > 0 { + fmt.Fprintf(w, "Refreshed the mxcli section of %s\n", strings.Join(res.Docs, ", ")) + } + if len(res.NoMarkers) > 0 { + fmt.Fprintf(errw, "Note: %s %s no mxcli section markers, so %s not refreshed. Run 'mxcli init' once to adopt them\n", + strings.Join(res.NoMarkers, ", "), map[bool]string{true: "has", false: "have"}[len(res.NoMarkers) == 1], + map[bool]string{true: "it was", false: "they were"}[len(res.NoMarkers) == 1]) + fmt.Fprintln(errw, " (it regenerates the file; keep project notes outside the markers from then on).") + } +} + +// mdlsourceReadme seeds mdlsource/, the directory the generated CLAUDE.md +// says scripts live in. Without it the first script had nowhere obvious to go. +const mdlsourceReadme = `# mdlsource + +MDL scripts for this project, one file per concern. Each starts with ` + "`mdl 1;`" + `. + + ./mxcli check mdlsource/.mdl -p .mpr + ./mxcli exec mdlsource/.mdl -p .mpr + +Exec a change twice: the second run must write nothing. +` + +// ensureMdlsourceDir creates mdlsource/ with a README so the directory +// survives git (which does not track empty directories). An existing README +// is left alone. +func ensureMdlsourceDir(projectDir string) (created bool, err error) { + dir := filepath.Join(projectDir, "mdlsource") + if err := os.MkdirAll(dir, 0o755); err != nil { + return false, err + } + readme := filepath.Join(dir, "README.md") + if _, err := os.Stat(readme); err == nil { + return false, nil + } + if err := os.WriteFile(readme, []byte(mdlsourceReadme), 0o644); err != nil { + return false, err + } + return true, nil +} diff --git a/cmd/mxcli/init_tooling_sync_test.go b/cmd/mxcli/init_tooling_sync_test.go new file mode 100644 index 0000000000..3f3af049c6 --- /dev/null +++ b/cmd/mxcli/init_tooling_sync_test.go @@ -0,0 +1,261 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// newSyncProject is a project as `mxcli init` left it, ready for a sync: an +// .mpr, a .claude/lint-rules directory, and the tooling dirs. +func newSyncProject(t *testing.T) string { + t.Helper() + dir := filepath.Join(t.TempDir(), "Demo") + for _, d := range []string{".claude/lint-rules", ".ai-context/skills"} { + if err := os.MkdirAll(filepath.Join(dir, d), 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(dir, "Demo.mpr"), nil, 0o644); err != nil { + t.Fatal(err) + } + return dir +} + +func mustWrite(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func mustRead(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(data) +} + +// ako/mxcli#952: `init --sync-skills` refreshed only the skills, so a project +// kept the lint rules of whichever mxcli first initialised it. The bundled +// rules are refreshed now; a project's own rule beside them is not touched. +func TestSyncProjectTooling_RefreshesBundledLintRulesOnly(t *testing.T) { + dir := newSyncProject(t) + names, err := bundledLintRuleNames() + if err != nil || len(names) == 0 { + t.Fatalf("no bundled lint rules: %v", err) + } + rulesDir := filepath.Join(dir, ".claude", "lint-rules") + stale := filepath.Join(rulesDir, names[0]) + mustWrite(t, stale, "# yesterday's rule\n") + user := filepath.Join(rulesDir, "zz_my_project_rule.star") + const userRule = "RULE_ID = \"PROJ001\"\ndef check():\n return []\n" + mustWrite(t, user, userRule) + + res, err := syncProjectTooling(dir) + if err != nil { + t.Fatal(err) + } + want, _ := lintRulesFS.ReadFile("lint-rules/" + names[0]) + if got := mustRead(t, stale); got != string(want) { + t.Errorf("bundled rule %s not refreshed", names[0]) + } + if got := mustRead(t, user); got != userRule { + t.Errorf("project rule was modified:\n%s", got) + } + if !strings.Contains(strings.Join(res.LintRules, ","), names[0]) { + t.Errorf("result does not report %s: %v", names[0], res.LintRules) + } + + // Control: a second sync has nothing to do. + res2, err := syncProjectTooling(dir) + if err != nil { + t.Fatal(err) + } + if len(res2.LintRules) != 0 || len(res2.Docs) != 0 || res2.Skills.Stale() || res2.StampWritten { + t.Errorf("second sync was not a no-op: %+v", res2) + } +} + +// A project without .claude/lint-rules (a tool that does not use them) does +// not grow one from the sync. +func TestSyncProjectTooling_NoLintRulesDirStaysAbsent(t *testing.T) { + dir := newSyncProject(t) + if err := os.RemoveAll(filepath.Join(dir, ".claude", "lint-rules")); err != nil { + t.Fatal(err) + } + if _, err := syncProjectTooling(dir); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(dir, ".claude", "lint-rules")); !os.IsNotExist(err) { + t.Error("sync created .claude/lint-rules") + } +} + +// The mxcli section of CLAUDE.md is refreshed; what the project wrote above +// and below the markers survives byte for byte. +func TestSyncProjectTooling_RefreshesOnlyTheMarkedSection(t *testing.T) { + dir := newSyncProject(t) + const above = "\n" + const below = "\n## Our conventions\n\nCustomers are never deleted.\n" + path := filepath.Join(dir, "CLAUDE.md") + mustWrite(t, path, above+wrapMxcliSection("# Mendix Project: Demo\n\nOld guidance from an older mxcli.\n")+below) + + res, err := syncProjectTooling(dir) + if err != nil { + t.Fatal(err) + } + got := mustRead(t, path) + want := above + wrapMxcliSection(generateClaudeMD("Demo", "Demo.mpr")) + below + if got != want { + t.Errorf("CLAUDE.md after sync:\n%s", got) + } + if strings.Contains(got, "Old guidance") { + t.Error("stale mxcli section survived") + } + if len(res.Docs) != 1 || res.Docs[0] != "CLAUDE.md" { + t.Errorf("Docs = %v, want [CLAUDE.md]", res.Docs) + } + // AGENTS.md did not exist and must not be created by a sync. + if _, err := os.Stat(filepath.Join(dir, "AGENTS.md")); !os.IsNotExist(err) { + t.Error("sync created AGENTS.md") + } +} + +// A CLAUDE.md from before the markers cannot be split into mxcli's text and +// the project's, so the unattended sync leaves it alone and says why. +func TestSyncProjectTooling_UnmarkedDocIsLeftAlone(t *testing.T) { + dir := newSyncProject(t) + path := filepath.Join(dir, "AGENTS.md") + const legacy = "# Mendix Project: Demo\n\nWritten by an mxcli without markers, then edited by hand.\n" + mustWrite(t, path, legacy) + + res, err := syncProjectTooling(dir) + if err != nil { + t.Fatal(err) + } + if got := mustRead(t, path); got != legacy { + t.Errorf("unmarked AGENTS.md was rewritten:\n%s", got) + } + var out, errOut bytes.Buffer + reportToolingSync(&out, &errOut, res) + if !strings.Contains(errOut.String(), "AGENTS.md has no mxcli section markers") { + t.Errorf("no notice about the missing markers:\n%s", errOut.String()) + } +} + +// The sync stamps the version it ran with. +func TestSyncProjectTooling_WritesStamp(t *testing.T) { + dir := newSyncProject(t) + withBinaryVersion(t, "v0.25.0", "2026-10-01T00:00:00Z") + res, err := syncProjectTooling(dir) + if err != nil { + t.Fatal(err) + } + if !res.StampWritten { + t.Error("stamp not written") + } + if s := readToolingStamp(dir); s == nil || s.Version != "v0.25.0" { + t.Errorf("stamp = %+v", s) + } +} + +// An older binary must not "refresh" the project back to what it knows: that +// would downgrade the skills, rules and CLAUDE.md a newer mxcli wrote. +func TestSyncProjectTooling_OlderBinaryRefuses(t *testing.T) { + dir := newSyncProject(t) + withBinaryVersion(t, "v0.25.0", "") + if _, err := syncProjectTooling(dir); err != nil { + t.Fatal(err) + } + names, _ := bundledLintRuleNames() + rule := filepath.Join(dir, ".claude", "lint-rules", names[0]) + mustWrite(t, rule, "# written by v0.25.0, newer than this binary knows\n") + + withBinaryVersion(t, "v0.24.0", "") + res, err := syncProjectTooling(dir) + if err != nil { + t.Fatal(err) + } + if res.RefusedFor == nil { + t.Fatal("older binary synced") + } + if got := mustRead(t, rule); !strings.Contains(got, "written by v0.25.0") { + t.Error("older binary overwrote the newer lint rule") + } + if s := readToolingStamp(dir); s.Version != "v0.25.0" { + t.Errorf("stamp downgraded to %s", s.Version) + } + var out, errOut bytes.Buffer + reportToolingSync(&out, &errOut, res) + if !strings.Contains(errOut.String(), "v0.24.0") || !strings.Contains(errOut.String(), "v0.25.0") { + t.Errorf("refusal does not name both versions:\n%s", errOut.String()) + } +} + +func TestMergeMxcliSection(t *testing.T) { + cases := []struct { + name, existing, want string + ok bool + }{ + {"no markers", "# hand written\n", "", false}, + {"begin only", mxcliSectionBegin + "\nx\n", "", false}, + {"end before begin", mxcliSectionEnd + "\n" + mxcliSectionBegin + "\n", "", false}, + {"replaced in place", "a\n" + wrapMxcliSection("old") + "b\n", "a\n" + wrapMxcliSection("new") + "b\n", true}, + {"older begin wording still matches", "\nold\n" + mxcliSectionEnd + "\ntail", wrapMxcliSection("new") + "tail", true}, + {"end marker at EOF without newline", mxcliSectionBegin + "\nold\n" + mxcliSectionEnd, wrapMxcliSection("new"), true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, ok := mergeMxcliSection(c.existing, "new") + if ok != c.ok || (ok && got != c.want) { + t.Errorf("merge = %q, %v; want %q, %v", got, ok, c.want, c.ok) + } + }) + } +} + +// ako/mxcli#952 item 4: CLAUDE.md says scripts live in mdlsource/, and init +// did not create it. Init also stamps the version and writes CLAUDE.md inside +// the markers; re-running it keeps the project's notes below them. +func TestInit_CreatesMdlsourceStampAndMarkedDocs(t *testing.T) { + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "Demo.mpr"), "") + runInit(t, []string{"claude"}, dir) + + if _, err := os.Stat(filepath.Join(dir, "mdlsource", "README.md")); err != nil { + t.Errorf("mdlsource/README.md not created: %v", err) + } + if readToolingStamp(dir) == nil { + t.Errorf("%s not written", toolingStampRel) + } + for _, doc := range []string{"CLAUDE.md", "AGENTS.md"} { + got := mustRead(t, filepath.Join(dir, doc)) + if !strings.HasPrefix(got, mxcliSectionBeginPrefix) || !strings.Contains(got, mxcliSectionEnd) { + t.Errorf("%s is not wrapped in the mxcli markers", doc) + } + } + + path := filepath.Join(dir, "CLAUDE.md") + notes := "\n## Project notes\n\nInvoices are immutable once sent.\n" + mustWrite(t, path, mustRead(t, path)+notes) + readme := filepath.Join(dir, "mdlsource", "README.md") + mustWrite(t, readme, "our own readme\n") + + runInit(t, []string{"claude"}, dir) + if got := mustRead(t, path); !strings.HasSuffix(got, notes) { + t.Errorf("re-running init dropped the project notes:\n%s", got) + } + if got := mustRead(t, readme); got != "our own readme\n" { + t.Error("re-running init overwrote mdlsource/README.md") + } +} diff --git a/cmd/mxcli/main.go b/cmd/mxcli/main.go index 2a2e7db586..fbe338c386 100644 --- a/cmd/mxcli/main.go +++ b/cmd/mxcli/main.go @@ -125,6 +125,16 @@ beta language; --mdl 0 starts them in the alpha language, and in the REPL an fmt.Fprintf(os.Stderr, "Using project: %s\n", discovered) } } + // A binary older than the mxcli that wrote this project's tooling is + // the cause of failures that otherwise read as project defects: a + // CLAUDE.md asking for statements this parser lacks, lint rules + // reading fields this binary does not expose (ako/mxcli#952). Once, + // on stderr, so it cannot corrupt --json or piped output. `init` + // reports the same skew itself, in the terms of what it will do. + if cmd != initCmd { + projectPath, _ = cmd.Flags().GetString("project") + warnIfToolingNewer(os.Stderr, projectPath) + } globalJSONFlag, _ = cmd.Flags().GetBool("json") globalMCPURL, _ = cmd.Flags().GetString("mcp") globalMCPDial, _ = cmd.Flags().GetString("mcp-dial") diff --git a/cmd/mxcli/tooling_stamp.go b/cmd/mxcli/tooling_stamp.go new file mode 100644 index 0000000000..d16fb7ddac --- /dev/null +++ b/cmd/mxcli/tooling_stamp.go @@ -0,0 +1,239 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "sync" + "time" +) + +// tooling_stamp.go records which mxcli wrote a project's tooling, and warns +// when a binary older than that one opens the project (ako/mxcli#952). +// +// The failure it exists for: a project's CLAUDE.md asked for `mdl 1;` and its +// lint rules read `document_noun`, both written by a newer mxcli than the +// v0.24.0 on PATH. Every one of those then failed — a parse error, three +// crashing rules counted as project errors — and nothing said the cause was the +// binary rather than the project, because nothing recorded which mxcli had +// written the tooling. +// +// A binary can only warn about a stamp it knows to read, so a binary that +// predates the stamp (v0.24.0 and everything before it) stays silent whatever +// the stamp says. That is why the bootstrap script checks the version too: it +// runs before the binary is chosen, and it is regenerated by the newer mxcli +// that writes the stamp. + +// toolingStampRel is the stamp's path relative to the project directory. It +// lives in .ai-context/, which every --tool receives, rather than .claude/, +// which only Claude Code projects have. +const toolingStampRel = ".ai-context/mxcli-tooling.json" + +// toolingStamp is the content of the stamp file. +type toolingStamp struct { + // Version is the mxcli version that last wrote the tooling, exactly as + // `mxcli --version` reports it (v0.25.0, nightly-20261002-4ba1495f2, + // v0.24.0-888-g4ba1495f2 for a dev build). + Version string `json:"version"` + // Built is that binary's build time (RFC 3339), when it had one. It is + // what lets a release and a nightly be ordered against each other. + Built string `json:"built,omitempty"` + // Written is the date the stamp was (re)written, YYYY-MM-DD. + Written string `json:"written"` +} + +// currentMxcliVersion is the running binary's version as the stamp records +// it. A build without -X main.Version has no meaningful version at all. +func currentMxcliVersion() string { + if Version == "" { + return "unknown" + } + return Version +} + +// readToolingStamp returns the project's stamp, or nil when there is none (or +// it is unreadable — a broken stamp must never stop a command). +func readToolingStamp(projectDir string) *toolingStamp { + data, err := os.ReadFile(filepath.Join(projectDir, toolingStampRel)) + if err != nil { + return nil + } + var s toolingStamp + if json.Unmarshal(data, &s) != nil || s.Version == "" { + return nil + } + return &s +} + +// writeToolingStamp records the running binary as the writer of the project's +// tooling. It rewrites nothing when the stamp already names this version, so a +// sync on every session start does not dirty the working tree once a day. +func writeToolingStamp(projectDir string) (changed bool, err error) { + cur := currentMxcliVersion() + if old := readToolingStamp(projectDir); old != nil && old.Version == cur { + return false, nil + } + s := toolingStamp{Version: cur, Built: BuildTime, Written: time.Now().UTC().Format("2006-01-02")} + data, err := json.MarshalIndent(s, "", " ") + if err != nil { + return false, err + } + path := filepath.Join(projectDir, toolingStampRel) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return false, err + } + if err := os.WriteFile(path, append(data, '\n'), 0o644); err != nil { + return false, fmt.Errorf("writing %s: %w", path, err) + } + return true, nil +} + +// versionKind classifies an mxcli version string. +type versionKind int + +const ( + versionUnknown versionKind = iota // dev build, untagged, or unparseable + versionRelease // v0.24.0 + versionNightly // nightly-20261002-4ba1495f2 +) + +type mxcliVersion struct { + kind versionKind + parts [3]int // release only + date string // nightly only: YYYYMMDD +} + +var ( + releaseVersionRe = regexp.MustCompile(`^v?(\d+)\.(\d+)\.(\d+)$`) + nightlyVersionRe = regexp.MustCompile(`^nightly-(\d{8})-[0-9a-f]+$`) + buildDateRe = regexp.MustCompile(`^(\d{4})-(\d{2})-(\d{2})T`) +) + +// parseMxcliVersion classifies a version. Anything that is not exactly a +// release tag or a nightly tag — `git describe` output such as +// v0.24.0-888-g4ba1495f2, a -dirty build, "unknown" — is versionUnknown, and an +// unknown version is never compared: a dev build is whatever its author made +// it, and guessing an order for it would warn about nothing or miss real skew. +func parseMxcliVersion(v string) mxcliVersion { + v = strings.TrimSpace(v) + if i := strings.IndexByte(v, ' '); i >= 0 { // "v0.24.0 (2026-…)" from --version + v = v[:i] + } + if m := releaseVersionRe.FindStringSubmatch(v); m != nil { + var out mxcliVersion + out.kind = versionRelease + for i := 0; i < 3; i++ { + out.parts[i], _ = strconv.Atoi(m[i+1]) + } + return out + } + if m := nightlyVersionRe.FindStringSubmatch(v); m != nil { + return mxcliVersion{kind: versionNightly, date: m[1]} + } + return mxcliVersion{} +} + +// buildDate turns an RFC 3339 build time into YYYYMMDD, or "". +func buildDate(built string) string { + m := buildDateRe.FindStringSubmatch(built) + if m == nil { + return "" + } + return m[1] + m[2] + m[3] +} + +// binaryOlderThan reports whether a binary (version + build time) is provably +// older than the one that wrote the stamp. "Provably" is the operative word: +// with either side unknown the answer is false. +// +// - two releases compare by version number; +// - two nightlies compare by the date in the tag; +// - a release against a nightly compares build dates, since neither number +// orders the other — and says nothing when a build date is missing. +// +// Dates are compared by day, so a release and a nightly from the same day are +// never reported as skewed. +// +// The bootstrap script carries a POSIX-sh copy of these rules (vclass / +// version_lt in bootstrapScriptTemplate); TestBootstrapVersionGuard holds the +// two together. +func binaryOlderThan(binVersion, binBuilt string, stamp toolingStamp) bool { + b := parseMxcliVersion(binVersion) + s := parseMxcliVersion(stamp.Version) + if b.kind == versionUnknown || s.kind == versionUnknown { + return false + } + if b.kind == versionRelease && s.kind == versionRelease { + for i := 0; i < 3; i++ { + if b.parts[i] != s.parts[i] { + return b.parts[i] < s.parts[i] + } + } + return false + } + bd, sd := b.date, s.date + if b.kind == versionRelease { + bd = buildDate(binBuilt) + } + if s.kind == versionRelease { + sd = buildDate(stamp.Built) + } + if bd == "" || sd == "" { + return false + } + return bd < sd +} + +// staleBinaryStamp returns the project's stamp when the running binary is +// older than it, else nil. +func staleBinaryStamp(projectDir string) *toolingStamp { + s := readToolingStamp(projectDir) + if s == nil || !binaryOlderThan(Version, BuildTime, *s) { + return nil + } + return s +} + +// updateTag is the release tag to fetch for a stamp version: a nightly is only +// ever available as the rolling "nightly" release. +func updateTag(stampVersion string) string { + if parseMxcliVersion(stampVersion).kind == versionNightly { + return "nightly" + } + return stampVersion +} + +// writeStaleBinaryWarning explains the skew: both versions, what goes wrong, +// and how to update. +func writeStaleBinaryWarning(w io.Writer, s toolingStamp) { + fmt.Fprintf(w, "Warning: this project's mxcli tooling (CLAUDE.md, skills, lint rules) was written by mxcli %s,\n", s.Version) + fmt.Fprintf(w, " but this binary is %s. Statements, commands and lint-rule fields the tooling uses may not exist here.\n", currentMxcliVersion()) + fmt.Fprintf(w, " Update: ./mxcli setup mxcli --tag %s --output ./mxcli (add --os/--arch off linux/amd64),\n", updateTag(s.Version)) + fmt.Fprintf(w, " or delete ./mxcli and re-run: sh %s\n", bootstrapScriptName) +} + +var toolingWarnOnce sync.Once + +// warnIfToolingNewer prints the skew warning at most once per process. It +// takes the -p value: a .mpr path or a project directory. +func warnIfToolingNewer(w io.Writer, projectPath string) { + if projectPath == "" { + return + } + dir := projectPath + if st, err := os.Stat(projectPath); err != nil || !st.IsDir() { + dir = filepath.Dir(projectPath) + } + s := staleBinaryStamp(dir) + if s == nil { + return + } + toolingWarnOnce.Do(func() { writeStaleBinaryWarning(w, *s) }) +} diff --git a/cmd/mxcli/tooling_stamp_test.go b/cmd/mxcli/tooling_stamp_test.go new file mode 100644 index 0000000000..5ec018e088 --- /dev/null +++ b/cmd/mxcli/tooling_stamp_test.go @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "sync" + "testing" +) + +// withBinaryVersion sets the running binary's version for one test. +func withBinaryVersion(t *testing.T, version, built string) { + t.Helper() + oldV, oldB := Version, BuildTime + Version, BuildTime = version, built + t.Cleanup(func() { Version, BuildTime = oldV, oldB }) +} + +// versionOrderCases is shared by the Go comparison and the bootstrap script's +// copy of it, so the two cannot disagree about what "older" means. +var versionOrderCases = []struct { + name string + have, haveBuilt string + want, wantBuilt string + older bool +}{ + {"release older", "v0.24.0", "", "v0.25.0", "", true}, + {"release older by minor across digits", "v0.9.0", "", "v0.10.0", "", true}, + {"release older by patch", "v0.24.0", "", "v0.24.1", "", true}, + {"release equal", "v0.24.0", "", "v0.24.0", "", false}, + {"release newer", "v0.25.0", "", "v0.24.0", "", false}, + {"release major beats minor", "v1.0.0", "", "v0.99.0", "", false}, + {"nightly older", "nightly-20261001-aaaaaaa", "", "nightly-20261002-4ba1495f2", "", true}, + {"nightly same day", "nightly-20261002-aaaaaaa", "", "nightly-20261002-4ba1495f2", "", false}, + {"nightly newer", "nightly-20261003-aaaaaaa", "", "nightly-20261002-4ba1495f2", "", false}, + {"release built before nightly", "v0.24.0", "2026-06-01T10:00:00Z", "nightly-20261002-4ba1495f2", "", true}, + {"release built after nightly", "v0.25.0", "2026-10-05T10:00:00Z", "nightly-20261002-4ba1495f2", "", false}, + {"nightly before release build", "nightly-20260601-aaaaaaa", "", "v0.25.0", "2026-10-05T10:00:00Z", true}, + {"release without build time vs nightly", "v0.24.0", "", "nightly-20261002-4ba1495f2", "", false}, + {"dev binary never older", "v0.24.0-888-g4ba1495f2", "2026-01-01T00:00:00Z", "v0.25.0", "", false}, + {"dirty binary never older", "v0.24.0-dirty", "", "v0.25.0", "", false}, + {"unknown binary never older", "unknown", "", "v0.25.0", "", false}, + {"dev stamp never newer", "v0.24.0", "", "v0.24.0-914-g5d6927a8e", "", false}, +} + +func TestBinaryOlderThan(t *testing.T) { + for _, c := range versionOrderCases { + t.Run(c.name, func(t *testing.T) { + got := binaryOlderThan(c.have, c.haveBuilt, toolingStamp{Version: c.want, Built: c.wantBuilt}) + if got != c.older { + t.Errorf("binaryOlderThan(%q, %q, %q/%q) = %v, want %v", c.have, c.haveBuilt, c.want, c.wantBuilt, got, c.older) + } + }) + } +} + +// The stamp is rewritten only when the version changes: the sync runs on +// every session start, and a "written" date that moved daily would dirty the +// working tree every day. +func TestWriteToolingStamp_RewritesOnlyOnVersionChange(t *testing.T) { + dir := t.TempDir() + withBinaryVersion(t, "v0.25.0", "2026-10-01T00:00:00Z") + + if changed, err := writeToolingStamp(dir); err != nil || !changed { + t.Fatalf("first write: changed=%v err=%v, want a write", changed, err) + } + s := readToolingStamp(dir) + if s == nil || s.Version != "v0.25.0" || s.Built != "2026-10-01T00:00:00Z" || s.Written == "" { + t.Fatalf("stamp = %+v", s) + } + if changed, _ := writeToolingStamp(dir); changed { + t.Error("second write with the same version changed the stamp") + } + withBinaryVersion(t, "v0.26.0", "") + if changed, _ := writeToolingStamp(dir); !changed { + t.Error("a new version did not rewrite the stamp") + } + if got := readToolingStamp(dir).Version; got != "v0.26.0" { + t.Errorf("stamp version = %q, want v0.26.0", got) + } +} + +// The reported skew: tooling written by a newer mxcli, opened by an older +// binary. The warning names both versions and how to update, and appears once +// however many times the check runs. +func TestWarnIfToolingNewer_OnceWithBothVersions(t *testing.T) { + dir := t.TempDir() + mpr := filepath.Join(dir, "App.mpr") + if err := os.WriteFile(mpr, nil, 0o644); err != nil { + t.Fatal(err) + } + withBinaryVersion(t, "v0.25.0", "") + if _, err := writeToolingStamp(dir); err != nil { + t.Fatal(err) + } + + toolingWarnOnce = sync.Once{} + t.Cleanup(func() { toolingWarnOnce = sync.Once{} }) + + // Control: the binary that wrote the stamp is silent. + var buf bytes.Buffer + warnIfToolingNewer(&buf, mpr) + if buf.Len() != 0 { + t.Fatalf("same version warned: %s", buf.String()) + } + + withBinaryVersion(t, "v0.24.0", "") + warnIfToolingNewer(&buf, mpr) + warnIfToolingNewer(&buf, dir) // a directory -p is accepted too + out := buf.String() + for _, want := range []string{"v0.25.0", "v0.24.0", "setup mxcli --tag v0.25.0", bootstrapScriptName} { + if !strings.Contains(out, want) { + t.Errorf("warning lacks %q:\n%s", want, out) + } + } + if n := strings.Count(out, "Warning:"); n != 1 { + t.Errorf("warned %d times, want once:\n%s", n, out) + } +} + +func TestWarnIfToolingNewer_DevBuildIsSilent(t *testing.T) { + dir := t.TempDir() + withBinaryVersion(t, "v0.25.0", "") + if _, err := writeToolingStamp(dir); err != nil { + t.Fatal(err) + } + toolingWarnOnce = sync.Once{} + t.Cleanup(func() { toolingWarnOnce = sync.Once{} }) + withBinaryVersion(t, "v0.24.0-888-g4ba1495f2", "") + var buf bytes.Buffer + warnIfToolingNewer(&buf, filepath.Join(dir, "App.mpr")) + if buf.Len() != 0 { + t.Errorf("a dev build warned: %s", buf.String()) + } +} diff --git a/docs-site/src/guides/marketplace.md b/docs-site/src/guides/marketplace.md index ef1f957d2b..97242be174 100644 --- a/docs-site/src/guides/marketplace.md +++ b/docs-site/src/guides/marketplace.md @@ -313,7 +313,7 @@ Re-running is free: a second run reports 0 units changed, because [idempotent wr |---|---|---| | `mxcli fix widgets` | **yes** | the fix — after any headless install | | `mxcli fix design-properties` | **yes** | the fix — after any headless install | -| `mxcli docker check` | no | runs the widget resync under a snapshot so the *check* is not tripped by CE0463; the stored model stays stale | +| `mxcli docker check` | no | runs the widget resync on a temporary copy so the *check* is not tripped by CE0463; the stored model stays stale (`--no-update-widgets` checks it as stored) | | `mxcli widget sync` | yes, partial | reconciles widget schemas in mxcli's own code; clears 7 of 40 on the reference fixture | CE6087 is distinct from `CE6083`, which is a *missing* design-property declaration and is fixed by installing everything the package ships — something `install` and `update` already do. diff --git a/docs-site/src/ide/init-output.md b/docs-site/src/ide/init-output.md index d26baaa540..c24a1b59e0 100644 --- a/docs-site/src/ide/init-output.md +++ b/docs-site/src/ide/init-output.md @@ -8,8 +8,11 @@ These files are shared by all AI tools: ``` your-mendix-project/ -├── AGENTS.md # Comprehensive AI assistant guide +├── AGENTS.md # Comprehensive AI assistant guide (mxcli section between markers) +├── mdlsource/ # MDL scripts, one file per concern +│ └── README.md ├── .ai-context/ +│ ├── mxcli-tooling.json # Which mxcli wrote this tooling (version stamp) │ ├── skills/ # MDL pattern guides │ │ ├── write-microflows.md # Microflow syntax and patterns │ │ ├── create-page.md # Page/widget syntax reference diff --git a/docs-site/src/ide/mxcli-init.md b/docs-site/src/ide/mxcli-init.md index 44b169417f..1c468b1298 100644 --- a/docs-site/src/ide/mxcli-init.md +++ b/docs-site/src/ide/mxcli-init.md @@ -64,6 +64,12 @@ All tools also receive the universal files (`AGENTS.md`, `.ai-context/`). 4. **Set up dev container** -- `.devcontainer/` with Dockerfile and configuration 5. **Copy mxcli binary** -- places the mxcli executable in the project root 6. **Install VS Code extension** -- copies and installs the bundled `.vsix` file +7. **Create `mdlsource/`** -- the directory the generated `CLAUDE.md` tells the assistant to keep MDL scripts in, with a short README (`mxcli new` creates it even with `--skip-init`) +8. **Stamp the version** -- `.ai-context/mxcli-tooling.json` records which mxcli wrote the tooling; see [Syncing with Updates](./syncing.md#the-version-stamp) + +`CLAUDE.md` and `AGENTS.md` are written between `` and +`` markers. Notes you add outside the markers survive a +re-run of `mxcli init` and every `mxcli init --sync-skills`. ## Adding a Tool Later diff --git a/docs-site/src/ide/syncing.md b/docs-site/src/ide/syncing.md index c09e052ecd..776e5511a0 100644 --- a/docs-site/src/ide/syncing.md +++ b/docs-site/src/ide/syncing.md @@ -1,28 +1,101 @@ # Syncing with Updates -When you upgrade `mxcli` to a newer version, the skills, commands, and VS Code extension bundled with it may have changed. This page explains how to keep your project's files in sync. +When you upgrade `mxcli`, the skills, lint rules and project guidance it ships +may have changed. A project keeps them in step with the binary through one +command, which the Claude Code bootstrap script runs on every session start: -## What Gets Updated +```bash +mxcli init --sync-skills # or: mxcli init --sync +``` -| Component | Source of Truth | Sync Target | -|-----------|----------------|-------------| -| Skills | `reference/mendix-repl/templates/.claude/skills/` | `.ai-context/skills/` | -| Commands | `.claude/commands/mendix/` | `.claude/commands/mendix/` | -| VS Code extension | `vscode-mdl/vscode-mdl-*.vsix` | Installed extension | -| Lint rules | Bundled Starlark rules | `.claude/lint-rules/` | +It is quiet when everything is already current. -## Re-running mxcli init +## What Gets Refreshed -The simplest way to sync is to re-run `mxcli init`: +| Component | Refreshed by `--sync-skills` | What is never touched | +|-----------|------------------------------|-----------------------| +| Skills | `.ai-context/skills/` and `.claude/skills/` | skill directories mxcli does not ship | +| Lint rules | the bundled rules in `.claude/lint-rules/`, identified by file name | any rule file under another name — your own rules | +| `CLAUDE.md`, `AGENTS.md` | the part between the `` and `` markers | everything outside the markers; a file that does not exist is not created | +| Version stamp | `.ai-context/mxcli-tooling.json` | — | -```bash -mxcli init /path/to/my-mendix-project +A bundled lint rule file belongs to mxcli, like a skill: an edit to it is +overwritten. To change a bundled rule, copy it under a new file name and rule +ID, or override its severity or options in `.claude/lint-config.yaml`. + +### The mxcli section of CLAUDE.md and AGENTS.md + +`mxcli init` writes its content between two markers. Put project notes **outside** +them — above or below — and every later sync and every re-run of `mxcli init` +keeps them: + +```markdown + +# Mendix Project: MyApp +… + + +## Our conventions + +Invoices are immutable once sent. ``` -This overwrites the generated skill files with the latest versions. **Custom modifications to built-in skill files will be lost.** To preserve customizations: +A file written by an mxcli from before the markers cannot be split into mxcli's +text and yours, so the sync leaves it alone and prints a note. Run `mxcli init` +once to adopt the markers: it regenerates the file, so move any notes you had +added below the end marker afterwards. + +## The Version Stamp -1. Keep custom skills in separate files (e.g., `my-custom-pattern.md`) -2. Or use version control to merge changes +`mxcli init` and every sync record the mxcli that wrote the tooling in +`.ai-context/mxcli-tooling.json`: + +```json +{ + "version": "v0.25.0", + "built": "2026-10-01T09:12:44Z", + "written": "2026-10-03" +} +``` + +It is rewritten only when the version changes, so the daily session-start sync +does not dirty the working tree. Commit it with the rest of the tooling. + +**An older binary warns.** Any command that opens the project (`-p`) with a +binary older than the stamp prints, once and on stderr, both versions and how +to update. The tooling may use statements, commands or lint-rule fields the +older binary does not have — `mdl 1;` is a parse error before it existed, and +a rule reading a newer catalog field fails. + +**An older binary does not sync.** `init --sync-skills` from an older binary +refuses rather than rolling the skills, rules and `CLAUDE.md` back to what it +knows. Update the binary instead. An explicit `mxcli init` from an older binary +still runs, after the same warning. + +Versions are compared only when the order is certain: two releases by number +(`v0.24.0` < `v0.25.0`), two nightlies by the date in the tag +(`nightly-20261002-…`), a release against a nightly by build date. A dev build +(`v0.24.0-888-g4ba1495f2`, `-dirty`, or no version) is never reported as older. + +### Binaries that predate the stamp + +A binary can only check a stamp it knows to read. mxcli v0.24.0 and earlier +open a project stamped by a newer mxcli **without any warning**. For them the +guard is the bootstrap script, `.claude/bootstrap-mxcli.sh`, which a newer +`mxcli init` regenerates: before it links the `mxcli` on `PATH` into the +project it compares that binary's version with the stamp, and it does not link +an older one — it downloads `MXCLI_TAG` (default `nightly`) instead. A +`./mxcli` older than the stamp is replaced the same way, through a temporary +file, so a `./mxcli` that is a symlink to the `PATH` binary never has the +download written through it. If the download fails, the older `./mxcli` is +kept with a warning rather than leaving the project without a binary. + +## Re-running mxcli init + +`mxcli init` regenerates everything, including the devcontainer and tool +configuration files, and keeps project notes outside the `CLAUDE.md` / +`AGENTS.md` markers. Use it after an upgrade that changed tool configuration; +for skills, rules and guidance alone, `--sync-skills` is enough. ## Build-Time Sync (for mxcli developers) @@ -37,15 +110,7 @@ make sync-vsix # VS Code extension only ## Checking Versions -To see which version of mxcli and its bundled assets you have: - ```bash -mxcli version +mxcli version # the binary +cat .ai-context/mxcli-tooling.json # the mxcli that wrote the project's tooling ``` - -## Recommended Workflow - -1. **Keep custom skills separate** from built-in skills so re-syncing does not overwrite them -2. **Use version control** for your `.ai-context/` and `.claude/` directories -3. **Re-run `mxcli init`** after upgrading mxcli to pick up new skills and bug fixes -4. **Review the diff** after syncing to see what changed in the skill files diff --git a/docs-site/src/tools/mxcli-lint.md b/docs-site/src/tools/mxcli-lint.md index 51cc2af09d..474b504fa5 100644 --- a/docs-site/src/tools/mxcli-lint.md +++ b/docs-site/src/tools/mxcli-lint.md @@ -89,6 +89,18 @@ mxcli lint -p app.mpr --format json | jq 'group_by(.severity) | map({severity: . mxcli lint -p app.mpr --format json | jq '[.[] | select(.severity == "error")]' ``` +## Rules That Fail + +A Starlark rule that reads a struct field this mxcli does not expose — usually a +rule written for a newer mxcli — is reported at **info** level as +`rule needs a newer mxcli ()`, not as an error. Any other rule +failure is an error (`Starlark rule error: …`). A rule's configured severity in +`lint-config.yaml` applies to its findings, never to its failure. `mxcli report` +keeps all rule failures out of the score. + +A rule file that does not load at all (for example, one calling a builtin this +mxcli lacks) is skipped with a warning on stderr. + ## Exit Codes | Code | Meaning | diff --git a/docs-site/src/tools/mxcli-report.md b/docs-site/src/tools/mxcli-report.md index ef06f1e059..8269a11f79 100644 --- a/docs-site/src/tools/mxcli-report.md +++ b/docs-site/src/tools/mxcli-report.md @@ -52,6 +52,19 @@ Each category shows: - Number of findings in that category - Specific rule violations with affected elements +## Rules That Could Not Run + +A lint rule that fails is a problem with the tooling, not the project, so it is +**not counted** in the score, the summary or any category. The report lists it +in its own section ("Rules That Could Not Run"; `ruleFailures` in JSON). + +The common case is a rule written for a newer mxcli — one that reads a field +this binary's catalog does not expose. It is reported at info level as +`rule QUAL004 needs a newer mxcli ("microflow" struct has no .document_noun_title attribute)`. +Any other failure is reported as a `Starlark rule error`. If the project's +tooling was written by a newer mxcli, `mxcli` warns about it on every command; +see [Syncing with Updates](../ide/syncing.md#the-version-stamp). + ## Writing Reports to Files ```bash diff --git a/docs-site/src/tutorial/skills.md b/docs-site/src/tutorial/skills.md index 8a120e80c8..464b95fa77 100644 --- a/docs-site/src/tutorial/skills.md +++ b/docs-site/src/tutorial/skills.md @@ -154,7 +154,8 @@ You can create your own skills to teach the AI about your project's patterns and ``` `mxcli init --sync-skills` only rewrites the skills mxcli itself ships, so your -own directories survive every upgrade. +own directories survive every upgrade. It also refreshes the bundled lint rules +and the mxcli section of `CLAUDE.md` — see [Syncing with Updates](../ide/syncing.md). A custom skill is a markdown document with two lines of frontmatter. Write the body the way you would explain something to a new team member, and write the diff --git a/mdl/catalog/catalog.go b/mdl/catalog/catalog.go index 516de09513..b3bf74c30c 100644 --- a/mdl/catalog/catalog.go +++ b/mdl/catalog/catalog.go @@ -6,6 +6,8 @@ package catalog import ( "database/sql" "fmt" + "os" + "path/filepath" "strconv" "strings" "time" @@ -348,6 +350,17 @@ func NewFromFile(path string) (*Catalog, error) { return nil, fmt.Errorf("failed to migrate cached catalog: %w", err) } + // A cache already at the current schema version was saved from a catalog + // that ran createTables, so it is complete — open it without writing. Opening + // must stay read-only: a parallel process may rename a fresh cache over this + // path at any moment (SaveToFile is atomic, ako/mxcli#951), and SQLite refuses + // a write to a file that has been moved ("attempt to write a readonly + // database"), while concurrent openers writing the version row contend for + // the lock. + if stored, err := c.GetMeta(MetaSchemaVersion); err == nil && stored == CatalogSchemaVersion { + return c, nil + } + // Idempotent schema upgrade — adds any tables/indexes the cached file // doesn't have yet. Existing data is untouched (unless dropped above). // createTables also records the current schema version in catalog_meta. @@ -443,6 +456,15 @@ func (c *Catalog) migrateIfSchemaMismatch() error { // SaveToFile saves the catalog to a SQLite file. // This copies the in-memory database to a file for persistence. // Requires the underlying CatalogDB to be a *SqliteCatalogDB. +// +// The save is atomic: the database is written to a temporary file in the same +// directory and renamed over path. Parallel mxcli processes on one project all +// save the same cache (ako/mxcli#951: eight parallel `lint` runs), and writing in +// place let them collide — VACUUM INTO refused the file another process had just +// created, the manual fallback then failed with "table catalog_meta already +// exists" or "database is locked", and a reader could open a half-written file. +// With a rename every writer succeeds (the last one wins) and a reader sees the +// old cache or a complete new one, never a partial file. func (c *Catalog) SaveToFile(path string) error { sdb, ok := c.db.(*SqliteCatalogDB) if !ok { @@ -450,15 +472,38 @@ func (c *Catalog) SaveToFile(path string) error { } rawDB := sdb.RawDB() + // Same directory, so the rename cannot cross a filesystem boundary. + tmpFile, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".tmp-*") + if err != nil { + return err + } + tmp := tmpFile.Name() + tmpFile.Close() + // VACUUM INTO requires the target to be absent or empty; it is empty. + committed := false + defer func() { + if !committed { + os.Remove(tmp) + } + }() + // Use SQLite backup API via VACUUM INTO (SQLite 3.27+) // Fall back to manual copy if not available - safePath := strings.ReplaceAll(path, "'", "''") - _, err := rawDB.Exec(fmt.Sprintf("VACUUM INTO '%s'", safePath)) - if err != nil { - // Fall back: export and import - return c.saveToFileManual(path, rawDB) + safePath := strings.ReplaceAll(tmp, "'", "''") + if _, err := rawDB.Exec(fmt.Sprintf("VACUUM INTO '%s'", safePath)); err != nil { + // Fall back: export and import, into a fresh empty temp file. + if err := os.Truncate(tmp, 0); err != nil { + return err + } + if err := c.saveToFileManual(tmp, rawDB); err != nil { + return err + } } + if err := os.Rename(tmp, path); err != nil { + return fmt.Errorf("replace catalog cache %s: %w", path, err) + } + committed = true return nil } diff --git a/mdl/catalog/catalog_save_race_test.go b/mdl/catalog/catalog_save_race_test.go new file mode 100644 index 0000000000..32262e43de --- /dev/null +++ b/mdl/catalog/catalog_save_race_test.go @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "fmt" + "os" + "path/filepath" + "sync" + "testing" + "time" +) + +// TestSaveToFile_ConcurrentWritersAndReaders is the guard for ako/mxcli#951 +// item 2. Eight parallel `mxcli lint` runs on a fresh project copy all built the +// catalog and saved it to the same .mxcli/catalog.db. The save removed the file +// and wrote into the path in place: VACUUM INTO refused a file another process +// had just created, the manual-copy fallback then hit "table catalog_meta already +// exists" or "database is locked", and a reader could open a half-written file. +// +// The save must be atomic: every writer succeeds, and a reader sees either the +// old cache or a complete new one — never a partial file. +func TestSaveToFile_ConcurrentWritersAndReaders(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "catalog.db") + + newCat := func(t *testing.T, mode string) *Catalog { + t.Helper() + cat, err := New() + if err != nil { + t.Fatalf("New: %v", err) + } + if err := cat.SetCacheInfo("/p/app.mpr", time.Unix(1700000000, 0), "11.8.0", mode, time.Second); err != nil { + t.Fatalf("SetCacheInfo: %v", err) + } + return cat + } + + // An existing cache is the common case (the project changed, so every + // process rebuilds and overwrites it). + seed := newCat(t, "fast") + if err := seed.SaveToFile(path); err != nil { + t.Fatalf("seed SaveToFile: %v", err) + } + seed.Close() + + const writers = 8 + cats := make([]*Catalog, writers) + for i := range cats { + cats[i] = newCat(t, "full") + } + defer func() { + for _, c := range cats { + c.Close() + } + }() + + stop := make(chan struct{}) + var readerErrs []error + var mu sync.Mutex + var readers sync.WaitGroup + for r := 0; r < 4; r++ { + readers.Add(1) + go func() { + defer readers.Done() + for { + select { + case <-stop: + return + default: + } + c, err := NewFromFile(path) + if err != nil { + mu.Lock() + readerErrs = append(readerErrs, err) + mu.Unlock() + continue + } + info, err := c.GetCacheInfo() + c.Close() + if err == nil && info.BuildMode != "fast" && info.BuildMode != "full" { + err = fmt.Errorf("reader saw build mode %q (partial cache)", info.BuildMode) + } + if err != nil { + mu.Lock() + readerErrs = append(readerErrs, err) + mu.Unlock() + } + } + }() + } + + start := make(chan struct{}) + errs := make(chan error, writers) + var wg sync.WaitGroup + for i := 0; i < writers; i++ { + wg.Add(1) + go func(c *Catalog) { + defer wg.Done() + <-start + errs <- c.SaveToFile(path) + }(cats[i]) + } + close(start) + wg.Wait() + close(stop) + readers.Wait() + close(errs) + + for err := range errs { + if err != nil { + t.Errorf("concurrent SaveToFile: %v", err) + } + } + for _, err := range readerErrs { + t.Errorf("concurrent reader: %v", err) + } + + final, err := NewFromFile(path) + if err != nil { + t.Fatalf("final cache does not open: %v", err) + } + defer final.Close() + info, err := final.GetCacheInfo() + if err != nil { + t.Fatalf("final cache info: %v", err) + } + if info.BuildMode != "full" { + t.Errorf("final cache build mode = %q, want full", info.BuildMode) + } + + // No temporary files may be left behind next to the cache. + entries, _ := os.ReadDir(dir) + for _, e := range entries { + if n := e.Name(); n != "catalog.db" && n != "catalog.db-journal" && n != "catalog.db-wal" && n != "catalog.db-shm" { + t.Errorf("leftover file next to the cache: %s", n) + } + } +} diff --git a/mdl/catalog/catalogdb_sqlite.go b/mdl/catalog/catalogdb_sqlite.go index 0da6e43bf2..cb7ec87d6e 100644 --- a/mdl/catalog/catalogdb_sqlite.go +++ b/mdl/catalog/catalogdb_sqlite.go @@ -6,6 +6,8 @@ package catalog import ( "database/sql" + "fmt" + "strings" _ "modernc.org/sqlite" ) @@ -28,9 +30,22 @@ func NewSqliteCatalogDB() (*SqliteCatalogDB, error) { return &SqliteCatalogDB{db: db}, nil } +// fileBusyTimeoutMs is how long a connection to an on-disk catalog waits for a +// lock held by another process before failing with SQLITE_BUSY. Opening a cache +// writes to it (schema upgrade, schema version), so parallel mxcli processes on +// one project contend for the write lock for a moment (ako/mxcli#951). +const fileBusyTimeoutMs = 10000 + // NewSqliteCatalogDBFromFile opens a file-based SQLite database. func NewSqliteCatalogDBFromFile(path string) (*SqliteCatalogDB, error) { - db, err := sql.Open("sqlite", path) + dsn := path + // The busy timeout goes in the DSN so it applies to every pooled connection, + // not only the one a PRAGMA statement happens to run on. A '?' in the path + // would be read as the start of the query string; keep the bare path then. + if !strings.Contains(path, "?") { + dsn = fmt.Sprintf("%s?_pragma=busy_timeout(%d)", path, fileBusyTimeoutMs) + } + db, err := sql.Open("sqlite", dsn) if err != nil { return nil, err } diff --git a/mdl/executor/cmd_catalog.go b/mdl/executor/cmd_catalog.go index 0415297b3f..9141297481 100644 --- a/mdl/executor/cmd_catalog.go +++ b/mdl/executor/cmd_catalog.go @@ -578,8 +578,9 @@ func buildCatalog(ctx *ExecContext, full, isSource, communities bool, resolution } cacheDir := filepath.Dir(cachePath) if err := os.MkdirAll(cacheDir, 0755); err == nil { - // Remove existing cache file first - os.Remove(cachePath) + // SaveToFile replaces the cache atomically. Do not remove it first: + // that opened a window in which a parallel process found no cache, + // and the in-place write that followed raced it (ako/mxcli#951). if err := cat.SaveToFile(cachePath); err != nil { fmt.Fprintf(ctx.progress(), "Warning: failed to save catalog cache: %v\n", err) } else { @@ -655,7 +656,6 @@ func execRefreshCatalogStmt(ctx *ExecContext, stmt *ast.RefreshCatalogStmt) erro return mdlerrors.NewBackend("graph analysis", err) } if !loadedFromCache && cachePath != "" { - os.Remove(cachePath) if err := ctx.Catalog.SaveToFile(cachePath); err != nil { fmt.Fprintf(ctx.Output, "Warning: failed to save catalog cache: %v\n", err) } diff --git a/mdl/linter/linter.go b/mdl/linter/linter.go index 2e1d0b1f2d..0141a8a96f 100644 --- a/mdl/linter/linter.go +++ b/mdl/linter/linter.go @@ -56,6 +56,11 @@ type Violation struct { Message string Location Location Suggestion string + // RuleFailure marks a finding about the rule rather than the project: the + // rule itself failed to run. It says nothing about the model, so the + // report keeps it out of the score and lists it separately + // (ako/mxcli#952), and a configured severity override does not apply. + RuleFailure bool } // Location identifies where a violation occurred. @@ -167,10 +172,14 @@ func (l *Linter) Run(ctx context.Context) ([]Violation, error) { // Run the rule violations := rule.Check(l.ctx) - // Apply configured severity if different from default + // Apply configured severity if different from default. A rule + // failure keeps its own: configuring QUAL004 as "warning" says how + // much its findings matter, not how much its crashing does. if config, ok := l.configs[rule.ID()]; ok { for i := range violations { - violations[i].Severity = config.Severity + if !violations[i].RuleFailure { + violations[i].Severity = config.Severity + } } } diff --git a/mdl/linter/report.go b/mdl/linter/report.go index 4337046890..48e30860f7 100644 --- a/mdl/linter/report.go +++ b/mdl/linter/report.go @@ -15,6 +15,10 @@ type Report struct { Categories []CategoryScore `json:"categories"` Violations []Violation `json:"-"` Summary Summary `json:"summary"` + // RuleFailures are the rules that failed to run. They are about the + // tooling, not the project, so they are neither in Violations nor in any + // score or count — they are listed on their own (ako/mxcli#952). + RuleFailures []Violation `json:"-"` } // CategoryScore tracks the score for a lint category. @@ -100,12 +104,26 @@ var categoryWeight = map[string]float64{ } // BuildReport creates a Report from a list of violations. -func BuildReport(projectName, date string, violations []Violation) *Report { +// +// A rule failure (Violation.RuleFailure) is split out into RuleFailures before +// anything is counted: the score measures the project, and a rule that could +// not run — usually one written for a newer mxcli — measured nothing. Counted, +// three crashing rules cost a project 30 points of "errors" it did not have. +func BuildReport(projectName, date string, all []Violation) *Report { + var violations, failures []Violation + for _, v := range all { + if v.RuleFailure { + failures = append(failures, v) + } else { + violations = append(violations, v) + } + } report := &Report{ - ProjectName: projectName, - Date: date, - Violations: violations, - Summary: Summarize(violations), + ProjectName: projectName, + Date: date, + Violations: violations, + Summary: Summarize(violations), + RuleFailures: failures, } // Group violations by category diff --git a/mdl/linter/report_format.go b/mdl/linter/report_format.go index 989140f890..89c0483031 100644 --- a/mdl/linter/report_format.go +++ b/mdl/linter/report_format.go @@ -5,6 +5,7 @@ package linter import ( "encoding/json" "fmt" + "html" "io" "strings" ) @@ -86,6 +87,17 @@ func (f *MarkdownReportFormatter) FormatReport(report *Report, w io.Writer) erro fmt.Fprintln(w) } + if len(report.RuleFailures) > 0 { + fmt.Fprintf(w, "## Rules That Could Not Run\n\n") + fmt.Fprintf(w, "Not counted in the score or the summary: these are problems with the lint tooling, not the project.\n\n") + fmt.Fprintf(w, "| Rule | Severity | Problem |\n") + fmt.Fprintf(w, "|------|----------|---------|\n") + for _, v := range report.RuleFailures { + fmt.Fprintf(w, "| %s | %s | %s |\n", v.RuleID, v.Severity, v.Message) + } + fmt.Fprintln(w) + } + return nil } @@ -115,6 +127,9 @@ type JSONReport struct { Summary JSONSummary `json:"summary"` Categories []CategoryScore `json:"categories"` Violations []JSONViolation `json:"violations"` + // RuleFailures are rules that could not run; excluded from the score and + // the summary (ako/mxcli#952). + RuleFailures []JSONViolation `json:"ruleFailures,omitempty"` } // JSONSummary is the summary in JSON format. @@ -152,6 +167,15 @@ func (f *JSONReportFormatter) FormatReport(report *Report, w io.Writer) error { }) } + for _, v := range report.RuleFailures { + jr.RuleFailures = append(jr.RuleFailures, JSONViolation{ + RuleID: v.RuleID, + Severity: v.Severity.String(), + Message: v.Message, + Suggestion: v.Suggestion, + }) + } + encoder := json.NewEncoder(w) encoder.SetIndent("", " ") return encoder.Encode(jr) @@ -274,6 +298,17 @@ func (f *HTMLReportFormatter) FormatReport(report *Report, w io.Writer) error { fmt.Fprintf(w, "\n") } + if len(report.RuleFailures) > 0 { + fmt.Fprintf(w, "

Rules That Could Not Run

\n") + fmt.Fprintf(w, "

Not counted in the score or the summary: these are problems with the lint tooling, not the project.

\n\n") + fmt.Fprintf(w, "\n") + for _, v := range report.RuleFailures { + fmt.Fprintf(w, "\n", + html.EscapeString(v.RuleID), v.Severity, html.EscapeString(v.Message)) + } + fmt.Fprintf(w, "
RuleSeverityProblem
%s%s%s
\n") + } + fmt.Fprintf(w, "\n\n") return nil } diff --git a/mdl/linter/starlark.go b/mdl/linter/starlark.go index ee623f6b64..769aa7242e 100644 --- a/mdl/linter/starlark.go +++ b/mdl/linter/starlark.go @@ -3,6 +3,7 @@ package linter import ( + "errors" "fmt" "os" "path/filepath" @@ -69,17 +70,51 @@ func (r *StarlarkRule) Check(ctx *LintContext) []Violation { // Call the check function result, err := starlark.Call(thread, r.checkFn, nil, nil) if err != nil { - return []Violation{{ - RuleID: r.id, - Severity: SeverityError, - Message: fmt.Sprintf("Starlark rule error: %v", err), - }} + return []Violation{ruleFailureViolation(r.id, err)} } // Convert result to violations return r.convertViolations(result) } +// missingStructAttrRe matches the evaluator's message for reading a field a +// struct does not have — "entity struct has no .document_noun attribute", +// optionally followed by a "(did you mean .x?)" hint. The evaluator flattens +// starlark.NoSuchAttrError into a plain error, so the message is all there is. +var missingStructAttrRe = regexp.MustCompile(`(?:\S+ )?struct has no \.[A-Za-z_][A-Za-z0-9_]* attribute.*`) + +// ruleFailureViolation reports a rule that failed to run. +// +// A rule that reads a struct field this binary does not expose is almost +// always a rule written for a newer mxcli: the shipped rules gain fields with +// the catalog (document_noun arrived after v0.24.0, and QUAL004, CONV010 and +// CUSTOM002 all crashed on it). That is reported as what it is — an info line +// naming the cause — rather than as a project error. Any other failure is a +// broken rule and stays an error. Both are RuleFailure: neither says anything +// about the project, so neither may move its score (ako/mxcli#952). +func ruleFailureViolation(ruleID string, err error) Violation { + msg := err.Error() + var evalErr *starlark.EvalError + if errors.As(err, &evalErr) { + msg = evalErr.Msg // without the backtrace + } + if detail := missingStructAttrRe.FindString(msg); detail != "" { + return Violation{ + RuleID: ruleID, + Severity: SeverityInfo, + Message: fmt.Sprintf("rule %s needs a newer mxcli (%s)", ruleID, detail), + Suggestion: "Update mxcli to the version that wrote this project's lint rules; if the rule is your own, check the field name against the write-lint-rules skill.", + RuleFailure: true, + } + } + return Violation{ + RuleID: ruleID, + Severity: SeverityError, + Message: fmt.Sprintf("Starlark rule error: %v", err), + RuleFailure: true, + } +} + // convertViolations converts a Starlark list to Go violations. func (r *StarlarkRule) convertViolations(result starlark.Value) []Violation { var violations []Violation @@ -1422,7 +1457,14 @@ func LoadStarlarkRulesFromDir(dir string) ([]*StarlarkRule, []RuleLoadFailure, e path := filepath.Join(dir, entry.Name()) rule, err := LoadStarlarkRule(path) if err != nil { - failures = append(failures, RuleLoadFailure{Path: path, Reason: err.Error()}) + reason := err.Error() + // A name the resolver does not know is, in a rule that used to + // load, a builtin from a newer mxcli — say so, as the run-time + // counterpart in ruleFailureViolation does (ako/mxcli#952). + if strings.Contains(reason, ": undefined: ") { + reason += " (a builtin this mxcli does not have — the rule may need a newer mxcli)" + } + failures = append(failures, RuleLoadFailure{Path: path, Reason: reason}) continue } diff --git a/mdl/linter/starlark_rule_failure_test.go b/mdl/linter/starlark_rule_failure_test.go new file mode 100644 index 0000000000..ed65e2c290 --- /dev/null +++ b/mdl/linter/starlark_rule_failure_test.go @@ -0,0 +1,189 @@ +// SPDX-License-Identifier: Apache-2.0 + +package linter + +import ( + "bytes" + "encoding/json" + "path/filepath" + "strings" + "testing" +) + +// ako/mxcli#952: a project's lint rules were written by a newer mxcli and read +// a struct field (document_noun) the binary on PATH did not expose. Three +// rules crashed, each crash was an error-severity finding, and the report +// scored the project 30 points lower for problems the project did not have. + +// newerMxcliRule reads a field no struct has — what a rule written for a newer +// mxcli looks like to an older one. +const newerMxcliRule = ` +RULE_ID = "NEWER001" +RULE_NAME = "Newer" +DESCRIPTION = "reads a field this mxcli does not expose" +CATEGORY = "Quality" +SEVERITY = "error" + +def check(): + v = violation(message = "x") + return [violation(message = v.document_noun_from_the_future)] +` + +// brokenRule fails for a reason that is not a missing field. +const brokenRule = ` +RULE_ID = "BROKEN001" +RULE_NAME = "Broken" +DESCRIPTION = "divides by zero" +CATEGORY = "Quality" +SEVERITY = "warning" + +def check(): + return [violation(message = str(1 // 0))] +` + +// findingRule is the control: a working rule whose finding must still count. +const findingRule = ` +RULE_ID = "QUAL001" +RULE_NAME = "Finding" +DESCRIPTION = "always finds one thing" +CATEGORY = "Quality" +SEVERITY = "warning" + +def check(): + return [violation(message = "a real finding")] +` + +func loadRuleSource(t *testing.T, name, src string) *StarlarkRule { + t.Helper() + dir := t.TempDir() + write(t, dir, name, src) + r, err := LoadStarlarkRule(filepath.Join(dir, name)) + if err != nil { + t.Fatalf("LoadStarlarkRule(%s): %v", name, err) + } + return r +} + +func TestStarlarkRule_MissingFieldIsNewerMxcliInfo(t *testing.T) { + r := loadRuleSource(t, "newer.star", newerMxcliRule) + vs := r.Check(&LintContext{}) + if len(vs) != 1 { + t.Fatalf("got %d violations, want 1: %+v", len(vs), vs) + } + v := vs[0] + if !v.RuleFailure { + t.Error("not marked as a rule failure") + } + if v.Severity != SeverityInfo { + t.Errorf("severity = %s, want info", v.Severity) + } + if !strings.HasPrefix(v.Message, "rule NEWER001 needs a newer mxcli (") || + !strings.Contains(v.Message, ".document_noun_from_the_future") { + t.Errorf("message = %q", v.Message) + } + if strings.Contains(v.Message, "Traceback") { + t.Errorf("message carries the backtrace: %q", v.Message) + } +} + +func TestStarlarkRule_OtherFailureStaysError(t *testing.T) { + r := loadRuleSource(t, "broken.star", brokenRule) + vs := r.Check(&LintContext{}) + if len(vs) != 1 || !vs[0].RuleFailure || vs[0].Severity != SeverityError { + t.Fatalf("got %+v, want one error-severity rule failure", vs) + } + if !strings.HasPrefix(vs[0].Message, "Starlark rule error:") { + t.Errorf("message = %q", vs[0].Message) + } +} + +// A severity configured for a rule is about its findings; it must not turn +// the rule's own failure back into an error (or hide a real crash). +func TestLinterRun_SeverityOverrideSkipsRuleFailures(t *testing.T) { + l := New(&LintContext{}) + l.AddRule(loadRuleSource(t, "newer.star", newerMxcliRule)) + l.ConfigureRule("NEWER001", RuleConfig{Enabled: true, Severity: SeverityError}) + vs, err := l.Run(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(vs) != 1 || vs[0].Severity != SeverityInfo { + t.Errorf("got %+v, want the failure to stay info", vs) + } +} + +func TestBuildReport_RuleFailuresAreNotScored(t *testing.T) { + l := New(&LintContext{}) + for name, src := range map[string]string{"newer.star": newerMxcliRule, "broken.star": brokenRule, "finding.star": findingRule} { + l.AddRule(loadRuleSource(t, name, src)) + } + all, err := l.Run(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(all) != 3 { + t.Fatalf("got %d violations, want 3: %+v", len(all), all) + } + + report := BuildReport("Demo", "today", all) + + // The control finding still counts: one warning in Quality. + if report.Summary.Total != 1 || report.Summary.Warnings != 1 || report.Summary.Errors != 0 { + t.Errorf("summary = %+v, want only the control warning", report.Summary) + } + if len(report.Violations) != 1 || report.Violations[0].RuleID != "QUAL001" { + t.Errorf("violations = %+v", report.Violations) + } + if len(report.RuleFailures) != 2 { + t.Errorf("rule failures = %+v, want both failing rules", report.RuleFailures) + } + // The score is the control finding's alone. + control := BuildReport("Demo", "today", []Violation{report.Violations[0]}) + if report.OverallScore != control.OverallScore { + t.Errorf("score = %v, want %v (rule failures must not move it)", report.OverallScore, control.OverallScore) + } + if control.OverallScore == 100 { + t.Error("control finding did not move the score; the comparison proves nothing") + } + + // Every format lists the failures separately. + var md bytes.Buffer + if err := GetReportFormatter("markdown").FormatReport(report, &md); err != nil { + t.Fatal(err) + } + if !strings.Contains(md.String(), "## Rules That Could Not Run") || !strings.Contains(md.String(), "needs a newer mxcli") { + t.Errorf("markdown lacks the rule-failure section:\n%s", md.String()) + } + var js bytes.Buffer + if err := GetReportFormatter("json").FormatReport(report, &js); err != nil { + t.Fatal(err) + } + var jr JSONReport + if err := json.Unmarshal(js.Bytes(), &jr); err != nil { + t.Fatal(err) + } + if len(jr.RuleFailures) != 2 || len(jr.Violations) != 1 { + t.Errorf("json: %d rule failures, %d violations; want 2 and 1", len(jr.RuleFailures), len(jr.Violations)) + } + var h bytes.Buffer + if err := GetReportFormatter("html").FormatReport(report, &h); err != nil { + t.Fatal(err) + } + if !strings.Contains(h.String(), "Rules That Could Not Run") { + t.Error("html lacks the rule-failure section") + } +} + +// A builtin a newer mxcli added fails at load, as an undefined name; the +// skipped-file reason says what that usually means. +func TestLoadStarlarkRulesFromDir_UndefinedBuiltinNamesNewerMxcli(t *testing.T) { + dir := t.TempDir() + write(t, dir, "future.star", "def check():\n return future_builtin()\n") + _, failures, err := LoadStarlarkRulesFromDir(dir) + if err != nil { + t.Fatal(err) + } + if len(failures) != 1 || !strings.Contains(failures[0].Reason, "may need a newer mxcli") { + t.Errorf("failures = %+v", failures) + } +}