fix(settings): stop ALTER SETTINGS dropping configuration data and silently ignoring invalid values - #56
Merged
Merged
Conversation
`bun install` floats typescript to ^6.0.2, which no longer auto-includes
every `@types/*` package on disk — `types` must be listed explicitly.
Without it tsc reported 43 errors on a clean checkout (`process`,
`console`, `setTimeout` unresolved, plus the implicit-any fallout on
node callbacks), so `make lint` failed before any change was made.
- tsconfig.json: declare `"types": ["node", "vscode"]`
- extension.ts: create the output channel with `{ log: true }` so it is a
`LogOutputChannel`, which is what vscode-languageclient 10's
`LanguageClientOptions.outputChannel` requires
CI only runs `make lint-go`, which is why this went unnoticed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012XR649rKk68z6gBpngu6MA
…em (mendixlabs#801) ALTER SETTINGS — any section, not just CONFIGURATION — serialized every ServerConfiguration from the semantic model, which carries only the fields mxcli understands. Everything else in the stored document was therefore silently deleted on write: - CustomSettings replaced with an empty array - Tracing replaced with null - OpenAdminPort / OpenHttpPort reset to false (the reader never populates them, so the model always held the zero value) - the Configurations / ConstantValues / CustomSettings version markers downgraded from 3 to the hardcoded 2 - constant overrides rewritten with a flat "Value", the shape Studio Pro and mxbuild ignore — so after one ALTER SETTINGS every override looked empty in Studio Pro, and Integer/Long constants failed the build The configurations are now overlaid onto the raw document they were read from (ADR-0005 guard-don't-drop, which the surrounding settings parts already followed): only fields the read path populates are written, each list keeps its stored marker, and a constant override is updated in the slot it already occupies so a nested SharedOrPrivateValue survives. A new override — which has no stored shape to preserve — is written nested, since that is what the platform reads. A configuration created by CREATE CONFIGURATION takes its shape from a sibling, with the per-configuration collections emptied and a fresh $ID. The overlay lives in mdl/settingsoverlay because both write engines had the same bug in duplicated form; sharing it keeps the codec engine (mdl/backend/modelsdk) and the legacy engine (sdk/mpr) from drifting again. Also refuse the write outright when no raw parts were captured on read: that path would have replaced every settings part with an empty array. Known limitation: a project whose overrides were already flattened by an earlier mxcli run keeps the flat shape, since the overlay preserves what is stored rather than converting it. Those overrides need to be re-entered in Studio Pro once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012XR649rKk68z6gBpngu6MA
…endixlabs#805) `alter settings configuration 'Default' HttpPortNumber = 'not-a-number'` printed "Updated configuration 'Default'" and changed nothing. Every Integer-typed setting parsed its value with the error discarded — `if v, err := strconv.Atoi(valStr); err == nil` — so an unparseable value skipped the assignment while the handler still reported success. DESCRIBE SETTINGS then showed the original value. The boolean settings had the same hole in a different form: `AllowUserMultipleSessions = valStr == "true"` mapped every other spelling, including a typo or a plausible 'yes', to false and reported success. Both now parse through helpers that return a validation error naming the setting and the offending value, so nothing is written. Covers all seven sites: BcryptCost, AllowUserMultipleSessions, DefaultTaskParallelism, WorkflowEngineParallelism, and HttpPortNumber / ServerPortNumber on both ALTER SETTINGS CONFIGURATION and CREATE CONFIGURATION. The same values are now reported at check time too (MDL-SET01 integers, MDL-SET02 booleans), wired into `mxcli check` and the LSP so a typo surfaces before the project is opened for writing. TestTypedSettingsKeys_MatchExecutor guards the check-time table against drifting from the executor's assignment switch. Out of scope: range validation. A port of 0 or 999999, or a negative BcryptCost, still parses as an integer and is accepted, as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012XR649rKk68z6gBpngu6MA
ako
pushed a commit
that referenced
this pull request
Jul 30, 2026
…ct arg (write is correct) Investigation of FINDINGS #56 ("describe page omits a show_page action's arguments"), verified against mxbuild 11.12.1: A widget button's show_page stores FormSettings.ParameterMappings as an empty list [2] and Mendix infers the current-row object for each unmapped page parameter. Storing an explicit `Argument: "$currentObject"` mapping makes mxbuild report CE0115 "arguments do not match" — the original issue mendixlabs#296, re-confirmed here. So the empty-mapping write is REQUIRED for a building app. Consequence: `show_page X` and `show_page X($p = $currentObject)` serialize to identical BSON, so describe→drop→exec re-produces a byte-identical valid page — the round-trip is functionally lossless; only the redundant $currentObject annotation is not echoed. No writer change (a fix there reintroduces CE0115); clarified the serializer comment and added a bug-test documenting the verified behavior and the boundary (a non-$currentObject widget page arg needs a Studio-Pro WidgetValue reference to encode). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two upstream issues in the project-settings write path. Both are in
ALTER SETTINGS, but they are independent bugs and land as separate commits.Issue numbers above refer to
mendixlabs/mxcli, not this fork.0b4feb7— overlay server configurations instead of rebuilding them (mendixlabs#801)serverConfigurationToBSONre-serialized eachSettings$ServerConfigurationfrommodel.ServerConfiguration, which carries only the fields mxcli models. Everything else in the stored document was therefore deleted on write. The reported symptoms plus two more found while reproducing:CustomSettingsemptiedTracingnulledValueinstead of the nestedSharedOrPrivateValuethe platform readsOpenAdminPort/OpenHttpPortreset tofalse— never populated on read, so the model always held the zero valueConfigurations/ConstantValues/CustomSettingsversion markers downgraded 3 → 2The configurations are now overlaid onto the raw document they were read from — the ADR-0005 guard-don't-drop form the surrounding settings parts already used. Only fields the read path populates get written, each list keeps its stored marker, and a constant override is updated in whichever slot it already occupies (nested if nested, flat if flat, per the issue's "maintain original storage shapes"). A brand-new override has no stored shape to preserve and is written nested.
CREATE CONFIGURATIONderives its shape from a sibling, empties the per-configuration collections, and mints a fresh$ID.Two things beyond the issue as filed:
sdk/mpr/writer_settings.go, same logic inbson.Dform).--engine legacyis a documented fallback, so leaving it would keep the same data loss reachable. Rather than duplicate the fix, the overlay is extracted intomdl/settingsoverlayand shared by both engines, so they cannot drift apart again.RawPartswould have replaced every settings part with an empty array, resetting the whole Project Settings dialog.readSettingsRawPartsreturnsnilon any read error, so this was reachable. Both engines now refuse the write instead.Known limitation, stated in the commit message: a project whose overrides were already flattened by an earlier mxcli run keeps the flat shape, because the overlay preserves what is stored rather than converting it. Those overrides need re-entering in Studio Pro once. Auto-upgrading flat → nested was deliberately left out: there is no ground truth on whether Mendix 9 legitimately stores flat, and no fixture in the repo has a Studio Pro-authored constant override to check against. With a real
ConstantValuefrom a 9.x and an 11.x project the repair could be made unconditional.4deca96— reject invalid typed values instead of ignoring them (mendixlabs#805)All seven
strconv.Atoisites discarded the parse error (if v, err := strconv.Atoi(valStr); err == nil), so an unparseable value skipped the assignment while the handler still printed its success line:BcryptCost,DefaultTaskParallelism,WorkflowEngineParallelism, andHttpPortNumber/ServerPortNumberon bothALTER SETTINGS CONFIGURATIONandCREATE CONFIGURATION.The boolean settings had the same hole in a different form:
AllowUserMultipleSessions = valStr == "true"mapped every other spelling — a typo, or a plausible'yes'— tofalseand reported success. The issue's "other typed configuration settings exhibiting similar behavior" covers it, so it is fixed here too.mxcli checkalso passed the invalid script cleanly, which is the same silent acceptance one layer up. AddedMDL-SET01(integers) andMDL-SET02(booleans), wired intomxcli checkand the LSP, so a typo surfaces before the project is opened for writing. That is also what makes the.fail.mdlregression fixture meaningful —make check-mdlonly runsmxcli check, so an executor-only fix would have had no CI gate.TestTypedSettingsKeys_MatchExecutorguards the new check-time table against drifting from the executor's assignment switch.Out of scope, stated in the commit message: range validation. A port of
0or999999, or a negativeBcryptCost, still parses as an integer and is accepted, as before.b87e1b9— makemake lintpass under TypeScript 6Unrelated to either issue, included because
make lintfailed on a clean checkout ofmainwith 43 TypeScript errors and CLAUDE.md requires it to pass.bun installfloatstypescriptto^6.0.2, which no longer auto-includes every@types/*package on disk — they must be listed intypes. Adding"types": ["node", "vscode"]left one genuine error:vscode-languageclient10 wants aLogOutputChannel, so the output channel is created with{ log: true }. CI only runsmake lint-go, which is why this went unnoticed.Drop this commit if you would rather keep the upstream PR to the two issues — it is self-contained and touches only
vscode-mdl/.Testing
make build,make test,make lintandmake check-mdlall pass.New coverage:
mdl/settingsoverlay/settingsoverlay_test.go— unit tests for the overlay: unknown-key preservation, marker preservation, the three constant-value shape paths, sibling-derived new configurations.mdl/backend/modelsdk/settings_write_configuration_test.go— round-trip against a seeded fixture (the fixture project has no overrides, custom settings or Tracing, so they are seeded first). Verified this test reproduces all six facets of ALTER SETTINGS (any command) silently corrupts every constant override in the project, and drops CustomSettings/Tracing, on write mendixlabs/mxcli#801 against the pre-fix code before the fix went in.mdl/executor/cmd_settings_validation_test.go— every rejection path asserts no write was attempted and no success line printed, plus the accept-valid cases (quoted number, numeric literal, padded, both booleans) so the fix does not over-reject.mdl-examples/bug-tests/801-alter-settings-preserves-configuration.mdl— repro with Studio Pro verification steps (needs a project, so it is a syntax-pass fixture only).mdl-examples/bug-tests/805-alter-settings-typed-values.fail.mdl— negative fixture, gated bymake check-mdl.Docs: two rows appended to the
.claude/skills/fix-issue.mdsymptom table,MDL-SET01/MDL-SET02registered indocs-site/src/tools/builtin-rules.md, and a note on typed values in theproject-settingsskill.🤖 Generated with Claude Code
https://claude.ai/code/session_012XR649rKk68z6gBpngu6MA
Generated by Claude Code