fix(catalog): bump the schema version for the two columns that skipped it

[jvegmond-tech]

Reply to post @ako's review on #1181
Thanks — the blocker is right, and chasing it turned up something worse.
The version bump. Correct on all counts: CREATE TABLE IF NOT EXISTS never adds a column, the guard only drops on a version difference, and the precedent (6291374) is exact. I should have followed it.
But #1181 is no longer the live case. Timeline in UTC on 25 Sept:

09:30 #1181 merged — UseRequestTimeout / TimeoutExpression, no bump
11:14 33fb2a9 bumps 12 → 14 (unrelated, mappings)
13:53 #1179 merged — DefaultMemberAccessRights, no bump

So the activity columns were rescued by accident: 14 landed after them and rebuilt those caches. The permissions column landed after 14 and is still broken on main today. PermissionsFor and Permissions bothSELECT COALESCE(DefaultMemberAccessRights, ''), so on any catalog built at 14 every permissions query fails with no such column. That takes out SEC001, SEC007 and CONV006/007/008 — and your point about mxcli report applies with more force there, since entity access rules do produce findings on a normal project, so the silent score divergence you couldn't demonstrate is reachable.

Both of those columns were mine, so this is one bug I shipped twice.
PR # bumps to 15 and documents both entries honestly — the accidental rescue included, so the next person reading the history doesn't conclude the gap never existed.
The test. Agreed the catalog-column precedent is the applicable one. Addedbuilder_rest_timeout_test.go, modelled on builder_audit_members_test.go: one ticked activity, one unticked, asserting both the toggle and the expression reach activities_data. It covers the case that motivated the columns — unticking "Use a timeout" leaves the seconds in place, so a rule keyed on the expression rather than the toggle passes a call that has no timeout.
Minor 1. You're right and I'll correct the record rather than the code — the case is applied in two loops; the rules loop passes constants, and a web-service activity in a rule is CE0009 anyway. Thanks for checking before flagging.
Minor 2. UseRequestTimeOut mirrors the storage name and the rest follow Go and Starlark idiom. The struct comment says so, but you're right that grepping one spelling misses the other. Happy to take a suggestion if you'd rather have one spelling throughout.
Minor 3. Agreed on its own issue — I'll open one formicroflow_write.go:1594 and microflow_webservice_write.go:154. Now that the value is read, wiring it through is a one-liner, but it changes what an MDL-authored REST call writes by default and that needs Mendix's own default settled first.
On reproducing. The relative-vs-absolute -p detail is a good catch and worth having in writing — a cache that silently rebuilds turns this class of bug invisible. Noted for the schema-version test I'd like to add next: a guard that fails when a column is added to createTables without the constant moving, so this stops depending on review catching it.

See task progress for longer tasks.
0001-fix-catalog-bump-the-schema-version-for-the-two-colu.patchreview-reply.md
mxclietest-main

[github-actions]

AI Code Review

Review Summary

This PR fixes a critical schema versioning oversight where two columns (UseRequestTimeout/TimeoutExpression in activities_data and DefaultMemberAccessRights in permissions_data) were added without incrementing the catalog schema version. The PR bumps the version to 15, adds a test verifying the RestCall timeout columns are properly stored, and documents the history.

What Looks Good

• Correct version bump: The schema version is incremented from 14 → 15 to match the actual schema changes, resolving silent query failures in caches built at version 14.
• Clear documentation: The history comment in tables.go explicitly explains why the bump was needed (both columns missed, activity columns accidentally rescued by v14, permissions column still broken).
• Targeted test: The new test (builder_rest_timeout_test.go) correctly models after existing catalog tests, verifying both timeout columns are stored and that the "off" case preserves the expression (critical for rule correctness).
• Minimal scope: Only touches exactly what's needed—version constant and history comment—plus a focused test. No unrelated changes.
• Follows precedent: Test structure mirrors builder_audit_members_test.go as suggested in review dialogue.

Minor Issues

• Test coverage gap: The test validates the activity columns but not the permissions_data.DefaultMemberAccessRights column. While the version bump fixes both, adding a similar test for the permissions column would provide symmetric coverage (though not strictly required since the version bump mechanism is validated via the activity test).
• History comment length: The explanation in tables.go is detailed but could be slightly trimmed for readability (e.g., moving some rationale to the PR description). However, this is acceptable given the complexity of the issue.

Checklist Compliance

• Overlap/duplication: None—this is a schema version fix, not new functionality.
• MDL syntax: N/A (no MDL changes).
• Full-stack consistency: N/A (no MDL feature added).
• Test coverage: ✅ New test added for the fixed functionality; follows existing patterns; no time.Sleep.
• Security/robustness: ✅ No injection risks; test uses static queries.
• Scope/atomicity: ✅ Single concern (schema version bump); test directly validates the fix.
• Code quality: ✅ Clean, consistent with existing patterns; no anti-patterns.
• Bugs/correctness: ✅ Fix addresses root cause (missing version bump); test confirms columns are accessible.

Recommendation

Approve. The PR correctly resolves the schema versioning bug with minimal, well-documented changes and adequate test coverage. The minor test gap for the permissions column is acceptable given the version bump mechanism is validated via the activity test, and the permissions column fix follows the same pattern. No blocking issues remain.

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source