Skip to content

Commit e39487a

Browse files
akoclaude
andcommitted
fix(modelsdk): emit grant execute for microflow/nanoflow allowed roles
DESCRIBE MICROFLOW/NANOFLOW on the modelsdk engine omitted the "grant execute on microflow … to <roles>;" line that the legacy engine emits. microflowFromGen/nanoflowFromGen never read the document's allowed module roles, so Microflow.AllowedModuleRoles was empty and the describer skipped the grant block entirely. Populate AllowedModuleRoles from the gen AllowedModuleRolesQualifiedNames() (BY_NAME role references, stored as qualified-name strings). Covered by an integration test against the fixture (Administration.ChangeMyPassword grants to Administrator + User). modelsdk now matches legacy byte-for-byte. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 19eaa93 commit e39487a

3 files changed

Lines changed: 54 additions & 0 deletions

File tree

.claude/skills/fix-issue.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ to the symptom table below, so the next similar issue costs fewer reads.
3232
| `DESCRIBE microflow` puts shared activities inside an `if … then` block — they should appear after `end if;` | Nested guard split inside `traverseFlowUntilMerge` crosses the outer merge boundary | `mdl/executor/cmd_microflows_show_helpers.go` — guard path in `traverseFlowUntilMerge` (~line 854) | Add `if contID != mergeID` guard before the `isMerge` skip-through so the guard continuation never crosses the outer merge |
3333
| `DESCRIBE microflow` on the **modelsdk** engine renders a branching microflow as `if cond then end if;` — the entire then/when body is dropped (legacy engine renders it fully). Affects projects Studio Pro saved with the older single-case storage | The split's branch case is stored under the BSON storage name `NewCaseValue` (single child), but (1) the codec had no `CaseValue``NewCaseValue` alias so it was never decoded, and (2) the adapter read only the plural `CaseValuesItems()`, not the singular `CaseValue()` | `modelsdk/codec/decoder.go` (`fieldAliases`) + `mdl/backend/modelsdk/microflow.go` (`caseValueFromGen`/`mapGenCaseValue`) | Add `"CaseValue": "NewCaseValue"` to `fieldAliases` (mirrors `Type``NewType`); read the singular `g.CaseValue()` first, then the plural list, mapping `EnumerationCase`/`InheritanceCase`. A round-trip MDL bug-test can't reproduce it (the write path emits the new format) — covered by codec + adapter unit tests |
3434
| `DESCRIBE microflow` on the **modelsdk** engine renders a rule-based split as `if true then …` instead of the real `Module.Rule_X(args)` condition (body renders, only the condition text is wrong) | `splitConditionFromGen` only handled `*ExpressionSplitCondition`; a `RuleSplitCondition` returned nil → renderer fell back to `true`. Even when handled, the rule name lives under the `RuleCall.Microflow` storage key, but gen decodes the property as `Rule` (and `ByNameRef` decode bypasses `fieldAliases`), so `RuleQualifiedName()` is empty | `mdl/backend/modelsdk/microflow.go` (`splitConditionFromGen`) | Reconstruct `*microflows.RuleSplitCondition` from the gen `RuleCall`; read the rule name from `rc.Raw().Lookup("Microflow")` when `RuleQualifiedName()` is empty (same raw-storage-key pattern the action readers use). The existing `formatSplitCondition` renders it |
35+
| `DESCRIBE microflow`/`nanoflow` on the **modelsdk** engine omits the `grant execute on microflow … to <roles>;` line that legacy emits | `microflowFromGen`/`nanoflowFromGen` never read the allowed module roles, so `Microflow.AllowedModuleRoles` was empty and the describer skipped the grant block | `mdl/backend/modelsdk/microflow.go` (`microflowFromGen`/`nanoflowFromGen`) | Populate `out.AllowedModuleRoles` from the gen `AllowedModuleRolesQualifiedNames()` (BY_NAME role refs, stored as qualified-name strings) |
3536
| MDL widget property `mxcli check`s clean but Studio Pro renders the default (e.g. `dataview ... (FormOrientation: Vertical)` always Horizontal) | V3 grammar generic-property branch parks the value in `w.Properties`, but the V3 builder never reads it and the writer never emits it; the widget struct has no field for it | `mdl/executor/cmd_pages_builder_v3_widgets.go` (`buildXxxV3`) + `sdk/mpr/writer_widgets_*.go` (`serializeXxx`) + `sdk/pages/pages_widgets_*.go` | Add field to `pages.Xxx`; read via `w.GetStringProp` / `w.GetIntProp`; write in `serializeXxx`. If the Studio Pro UI label differs from the BSON storage name (e.g. DataView "Form Orientation" → `LabelWidth: 0/N`), confirm by diffing a Studio Pro-saved page against the reflection-data defaults |
3637
| `ALTER <DOC TYPE> ...` parse error "no viable alternative at input 'ALTER<TYPE>'" but `CREATE <DOC TYPE> ...` works | The grammar lists ALTER variants explicitly per document type; a new doc type was added to `createStatement` but not to `alterStatement` | `mdl/grammar/MDLParser.g4` `alterStatement` block + dedicated visitor + new `Alter*Stmt` AST type + `register_stubs.go` | Add ALTER rule (mirror the closest sibling — `ALTER ODATA CLIENT` for SET-only, `ALTER PUBLISHED REST SERVICE` for SET+ADD/DROP); regenerate grammar; add `Alter*Stmt` with `Changes map[string]string`; route via `exitAlterStatement` dispatcher; register handler. Add the new AST type to `registry_test.go`'s `allKnownStatements()` |
3738
| Pluggable widget datasource property (`optionsSourceAssociationDataSource: Module.Entity`) passes `check` + `exec` but `mx check` reports CE0642 "Property 'Entity' is required" | A `datasource`-operation property was authored by name; the engine reads the widget's `datasource:` clause (`Properties["DataSource"]`), not the named key, so the value is silently dropped (and `hasDataSource` mode-selection also misfires) | `mdl/executor/validate_widgets.go` (`datasourceTypedKeys`) — and author via the `datasource:` clause | `check` now rejects named datasource-typed props (MDL-WIDGET05); the fix for the user is `datasource: database Module.Entity`. Persisting a *named* datasource property (multi-datasource widgets) needs a Studio-Pro-verified mapping — deferred. Real-but-unmapped props get MDL-WIDGET06 warning via def.json `knownProperties` (not a false MDL-WIDGET01) |

mdl/backend/modelsdk/microflow.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ func nanoflowFromGen(nf *genMf.Nanoflow, containerID model.ID) *microflows.Nanof
8282
ReturnType: dataTypeFromGen(nf.MicroflowReturnType()),
8383
}
8484
out.ID = model.ID(nf.ID())
85+
for _, qn := range nf.AllowedModuleRolesQualifiedNames() {
86+
out.AllowedModuleRoles = append(out.AllowedModuleRoles, model.ID(qn))
87+
}
8588
params, objs := splitFlowObjects(nf.ObjectCollection())
8689
out.Parameters = params
8790
flows := flowsFromGen(nf.FlowsItems())
@@ -102,6 +105,11 @@ func microflowFromGen(mf *genMf.Microflow, containerID model.ID) *microflows.Mic
102105
ReturnType: dataTypeFromGen(mf.MicroflowReturnType()),
103106
}
104107
out.ID = model.ID(mf.ID())
108+
// AllowedModuleRoles (BY_NAME role references) — without these DESCRIBE omits
109+
// the "grant execute on microflow … to …" line that legacy emits.
110+
for _, qn := range mf.AllowedModuleRolesQualifiedNames() {
111+
out.AllowedModuleRoles = append(out.AllowedModuleRoles, model.ID(qn))
112+
}
105113
params, objs := splitFlowObjects(mf.ObjectCollection())
106114
out.Parameters = params
107115
// Flows live on the gen Microflow, but the model keeps them in the object
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
package modelsdkbackend
4+
5+
import "testing"
6+
7+
// TestMicroflowFromGen_ReadsAllowedModuleRoles guards the "grant execute on
8+
// microflow … to …" line in DESCRIBE MICROFLOW. The allowed module roles are
9+
// BY_NAME references the gen exposes via AllowedModuleRolesQualifiedNames();
10+
// the adapter must surface them as Microflow.AllowedModuleRoles or DESCRIBE
11+
// drops the grant line that the legacy engine emits.
12+
func TestMicroflowFromGen_ReadsAllowedModuleRoles(t *testing.T) {
13+
b := New()
14+
if err := b.Connect(fixture); err != nil {
15+
t.Fatalf("connect: %v", err)
16+
}
17+
t.Cleanup(func() { _ = b.Disconnect() })
18+
19+
mfs, err := b.ListMicroflows()
20+
if err != nil {
21+
t.Fatalf("ListMicroflows: %v", err)
22+
}
23+
var roles []string
24+
for _, mf := range mfs {
25+
if mf.Name == "ChangeMyPassword" {
26+
for _, r := range mf.AllowedModuleRoles {
27+
roles = append(roles, string(r))
28+
}
29+
}
30+
}
31+
if roles == nil {
32+
t.Fatal("ChangeMyPassword not found or has no AllowedModuleRoles")
33+
}
34+
want := map[string]bool{"Administration.Administrator": false, "Administration.User": false}
35+
for _, r := range roles {
36+
if _, ok := want[r]; ok {
37+
want[r] = true
38+
}
39+
}
40+
for role, seen := range want {
41+
if !seen {
42+
t.Errorf("AllowedModuleRoles missing %q (got %v)", role, roles)
43+
}
44+
}
45+
}

0 commit comments

Comments
 (0)