Edit roles in a visual editor, with JSON as the fallback

[dawsontoth]

Stacked on #1875 (base claude/1299-role-attribute-permissions). Review that one first; this diff is only the role editor.

⊙ Problem

The role editor is a JSON editor (#1634). Edit Role opens Monaco on a template that calculateDefaultPermissions builds from describe_all. The only structured control is the operations allowlist lens above it. Add Role takes a name and two checkboxes, and you set table permissions afterwards in the JSON.

Two defects come with that:

• The template rewrites roles. Its single "Pick Attributes" checkbox expands or empties every table's attribute list at once. When it expands a scoped table, it writes the primary key as an explicit false row. Harper otherwise derives the key's access from the other attributes (permissionsTranslator.js getTableAttrPerms), so opening and saving a scoped role revoked its primary key.
• Add Role can't create a super user. The form always sends structure_user next to super_user. Harper's validateNoSUPerms rejects any second key next to super_user: true. Against the 5.3.1 build, {super_user: true, structure_user: false} returns "Roles with 'super_user' set to true cannot have other permissions set."

❓ Your call: Is this the right first slice of #1634? It covers role type, structure user, the existing operations editor, table CRUD and attribute scoping, in both Add and Edit, with JSON kept as a fallback. What it leaves out is listed under Solution. It says Refs #1634 rather than closing it. The alternative was to ship Edit only and leave Add for later. I kept Add because unifying the two is half the issue and fixes the super-user bug for free. Splitting Add back out is a revert of one modal. The adjudicator's do-less option goes further: land only the template removal first, with Monaco on the stored JSON, which alone stops the rewrite, and the GUI after it.

💡 Solution

Both modals open on a visual editor, and Edit as JSON switches to the same draft in Monaco. This follows the schema editor's toggle (SchemaEditorView).

• Role type: Standard / Super user / Cluster user. The cluster user option is offered on Harper 4. On Harper 5, 5.0.0 prereleases included, it is offered only to a role that already has the flag, because nothing in Harper 5 core or harper-pro reads it.
• Structure user: No / All databases / Selected databases, with a database picker.
• Operations: the existing OperationsAllowlistEditor, unchanged and still version-gated.
• Table permissions: a collapsible section per database, with read/insert/update/delete per table and a tri-state "all tables" row.
• Attribute scoping: per table, a "Limit to specific attributes" switch and a read/insert/update grid that shows the access Harper actually applies. That covers the derived primary key and timestamps being read-only. Before the switch is flipped, the panel says that limiting lists only the attributes describe_all shows, so a computed or relationship attribute it leaves out loses access until granted in the JSON.

The JSON text stays the only source of truth. Every visual edit is a copy-on-write patch of the keys it touches, so content the GUI does not model passes through unchanged. Examples are a database record's describe and an attribute entry's delete. That content is listed as "kept as they are". The visual view is offered only while the document is one it can represent. Otherwise the JSON view lists the reasons, for example a non-boolean CRUD flag or an unusable operations value. Edit no longer seeds a template, so tables the role doesn't mention stay out of the JSON, and the GUI shows them as no access.

The edit modal also stops two lost-update paths. Its draft is seeded once, so the modal is keyed per role: moving straight from one role to another used to keep the first draft. And because alter_role replaces the whole permission with no revision check, a newer polled __updatedtime__ blocks Save until you Load the latest version or Keep my changes.

Every claim in the new copy was traced to Harper source: what structure_user grants, super user bypassing table permissions, an unchecked attribute having no access, the derived primary key, timestamps being read-only, and the cluster user flag on Harper 4 storing a recoverable password (harperdb security/user.ts) and listing every database, table and attribute in describe_all (dataLayer/schemaDescribe.js:36). The table-level rules were then run through the 5.3.1 build (Verification).

Not covered yet, still JSON-only: granting @computed and relationship attributes, because describe_all omits them (#1818 adds the include_computed option), legacy pre-2.2 attribute_restrictions roles, and a database record's describe flag, which is preserved but not shown. There is no table filter for very large schemas either.

⚖️ Alternatives

• A typed model with a serializer, like the schema editor's reducer. Rejected: serializing from a model drops every key the model doesn't know. Patching the parsed JSON keeps them by construction.
• Keep the template and add more lenses above Monaco. This was the planning review's do-less option. Rejected: the template is the lossy part, so every lens over it inherits the loss, and the issue asks for the JSON to be hidden by default.
• A Harper-side "normalized role" operation. Rejected: none exists, and Studio edits roles on 4.x and 5.0–5.3.

❓ Your call: Should Save be blocked while the role names a database or table that describe_all doesn't list? I left it as a warning with a Remove them button. Harper does reject such a save (SCHEMA_NOT_FOUND / TABLE_NOT_FOUND), and its 400 names the entry. But describe_all is cached for 60s and shared with the Databases tab, so a database created a moment ago reads as missing, and a block would refuse a save Harper accepts. Codex asked for the block. The Harper domain adjudicator accepted the warning as a known limit and suggested a middle path: refetch describe_all on Save, then block. Adding the block is one condition on the Save button.

❓ Your call: Should Harper 4 offer the cluster user type only to a role that already has it, as Harper 5 does? The adjudicator recommends it. A new role with the flag gets nothing useful from it. Harper 4 clustering recognizes only a role named cluster_user (security/user.ts:450, server/nats/utility/natsConfig.js:93), and the installer creates that one. So the flag only lists the whole schema to the role's users and stores their passwords in a recoverable form. Restricting it is one condition on offerClusterUser.

Codex, and at first the adjudicator, said a Harper 4 cluster user is super-user-equivalent. It isn't, in the 4.7.39 source. The translator's "full CRUD" comment returns the permission untranslated, and nothing downstream grants from it. The operations gate and SQL short-circuit only on super_user (utility/operation_authorization.js:274,370,499), and so does REST's getTablePermissions (resources/Table.ts:3643). Native replication admits only a super user or a replicates user (server/replication/replicationConnection.ts:1031). A role with no databases named then fails SCHEMA_NOT_FOUND. Both reviewers dropped the finding on the delta round. The copy names what the flag does: the describe_all listing (dataLayer/schemaDescribe.js:36) and the recoverable password.

❓ Your call: Should I file a Harper issue for a revision precondition on alter_role? The modal's __updatedtime__ check only sees what the roles list's 10s poll has fetched, so a write that lands inside that window is still overwritten. The adjudicator calls the client check a stopgap and the precondition the durable fix.

🔧 Changes

• The model, in rolePermissionModel.ts, is pure and unit-tested. Every write goes through own-key helpers that rebuild objects with Object.fromEntries, because Harper permits a database, table or attribute named __proto__, and an assignment would hit the prototype setter. Its parts:
  • getVisualEditorBlockers decides when only the JSON view can show a document. A table entry with no attribute_permissions gets a blocker that says how to fix it.
  • getPreservedEntries lists content kept but not shown, with "not in this instance's schema" kept separate so removeEntriesNotInSchema can remove exactly that.
  • setRoleType writes a flag being turned off as false only where the key already exists.
  • Revoking a table action also revokes it on every attribute entry, including entries the schema doesn't show, because role validation rejects an attribute granting more than its table (MISMATCHED_TABLE_ATTR_PERMS). setDatabaseAction does a whole database in one rebuild.
  • The derived primary key counts every listed attribute.
  • setAttributeScoping lists every attribute with the table's own access when switched on, so nothing changes at that moment. When it restores a list switched off this session, it clamps that list to the table.
  • setAttributeAction adds an unlisted attribute with its effective access first, and also grants the table an action given to one of its attributes.
• The draft, in useRolePermissionDraft.ts, keeps the object a visual edit produced instead of re-parsing its own text. Untouched databases and tables keep their identity.
• The shell, in RolePermissionEditor.tsx, loads describe_all and the version. isOperationsValueUnusable blocks Save on an operations value that save would drop. Without a version, that means anything but a list of names. The JSON view defers its blocker check while you type, and the initial view is decided once the schema loads. A schema that doesn't load leaves the JSON view in charge, with Retry, and an unusable operations value still gets the existing notice.
• The visual editor, in RoleVisualEditor.tsx:
  • It shows the schema-missing warning with Remove them and the "kept as they are" list.
  • It has version-specific cluster user copy and the structure-user modes.
  • One database opens by default, not every granted one.
  • A single stable updater, pointed at the committed document from a layout effect rather than during render, feeds the memoized DatabaseSection and TableRows.
  • The attribute panel carries the scoping copy and the notes on the derived primary key and timestamps.
• Modals:
  • EditRoleModal.tsx seeds the draft from the stored permission, announces a newer stored version and blocks Save until it is resolved. It remounts the editor when the latest version is loaded.
  • AddRoleModal.tsx uses the same editor and sends the prepared permission in one add_role.
  • index.tsx keys the edit modal per role, and mounts Add only while it is open, so a closed Add fetches nothing and every open starts fresh. Adding a role no longer opens the edit modal afterwards.
• Save: preparePermissionForSave.ts drops structure_user beside super_user/cluster_user, which validateNoSUPerms rejects.
• API: addRole.ts takes a full permission instead of the two booleans, and addRoleFormSchema.ts drops them from the form schema.
• Removed: defaultCalculator.ts and its test. Nothing else used the template.
• Docs: the roles DESIGN.md records the text-as-truth invariant, what forces the JSON view, what Save changes on purpose, and the draft and conflict rules. It is indexed from the root DESIGN.md.

✅ Verification

• Route: tests, plus a contract check against Harper's own build. Live Harper was not run end to end. I ran Harper 5.3.1's role_validation and permissionsTranslator from the harper checkout's dist, with getDatabases and hdb_schema stubbed to a fixture schema, over the payloads the editor produces. A new table grant, scoping turned on (timestamps read-only), preserved describe and attribute delete, structure_user: ['data'] with table grants elsewhere, and structure_user: true with an allowlist all validate. An attribute grant above its table and a table missing from the schema are rejected, which is what the editor guards against and warns about. The translator's effective access matches what getAttributeViews shows in three cases: a derived key (id = read only, from name; timestamp insert ignored), a key made explicit, and a key derived from a listed attribute describe_all doesn't show. It also confirms that a structure_user database gets full CRUD whatever its table entries say. The add-role shapes were checked the same way: {super_user: true, structure_user: false} is rejected, and {super_user: true} and {} pass. Off-repo, so reviewers can't re-run it.

• Tests, from npx vitest run src/features/instance/config/roles/ (127 passed). The visual editor and both modal suites render under StrictMode, as main.tsx does:

  • rolePermissionModel.test.ts pins every blocker and preserved-entry category, and every updater keeping untouched keys. That includes __proto__ names, the derived primary key and read-only timestamps, and a key derived from an attribute describe_all doesn't show.
  • RoleVisualEditor.test.tsx covers the tri-state database row, no restored attribute grant exceeding its table, the database picks restored after a structure-user mode switch, the cluster user copy per version, no cluster user type for a new role on a 5.0.0 prerelease or an unparseable version, the hidden-attribute caveat shown before scoping is switched on and only one database opening.
  • EditRoleModal.test.tsx drives the real modal through alter_role:
    • unmodelled content surviving an edit;
    • one draft shared across the two views;
    • blocked documents keeping the JSON view in charge;
    • a role opened in the JSON view staying there once fixed, until the author picks the visual editor;
    • the schema-load fallback and the unknown-version operations guard;
    • the conflict flow;
    • the operations wiring moved over from the old JSON-first tests.
  • AddRoleModal.test.tsx checks that a super user role is created as {super_user: true}.
  • index.test.tsx switches roles while the editor is open and checks that Save sends the second role's own permission.
  • preparePermissionForSave.test.ts covers structure_user and false flags beside a super/cluster flag.

• Mutation checks: I reverted each new guard in turn, 19 in all, and each turned at least one test red. The last three are from the adjudicated round: the old Harper 4 check, the pre-toggle caveat and the cluster user copy. For the gemini-code-assist follow-ups, removing the structure-user stash, removing the layout-effect write, and applying gemini's derived view state each turn a test red. Removing the guard on the initial-view setPreferJson makes 22 modal tests fail with "Too many re-renders".

• Large schema: with 5 databases, 500 tables, 30 attributes each and every table granted, a checkbox click re-renders in about 5ms under the React Profiler in jsdom. Without the per-database memo it took 83ms. After the updater's ref moved into a layout effect, a click on a 500-table database still re-renders exactly one TableRows, at about 3ms in jsdom before and after.

• Gates: the full vitest run passed (398 files, 3717 tests, 11 skipped). tsc -b, oxlint and dprint check all exited 0. The pre-commit hook ran on every commit without --no-verify. Browser verification was skipped because port 5173 is shared with parallel sessions.

• Cross-model review: six rounds after a codex planning review of the design (verdict chosen-approach-sound). The Cursor legs could not fetch through the 1Password SSH agent in rounds 1-3 and were not selected in rounds 4-6.

  • Rounds 1-3 (one full, two delta) had codex and gemini only, because the Harper domain (adjudication) leg's OAuth session had expired. I triaged those findings by hand and fixed the role-switch draft reuse, the save over a newer version, the hidden-attribute key derivation, the unbounded initial expansion, the cluster-user copy and the unknown-version operations save.
  • Round 4 was a full round with codex, gemini and the Harper domain adjudicator, which rated the branch major. I fixed what it kept in e603b0f0. 5.0.0 prereleases and unparseable versions were offered the cluster user type as if they were Harper 4. The warning that scoping drops computed and relationship attributes appeared only after the switch was flipped. Two doc comments contradicted the code: the notInSchema one said those entries block Save, and the scoping one said access doesn't change. Its cluster-user major is answered in the cluster user Your call above.
  • Round 5 was a delta over that commit with the same three legs. The adjudicator withdrew the cluster-user major and rated the branch nit.
  • Round 6 was a delta over the gemini-code-assist follow-ups below, with the same three legs. It found no new defect and rated the branch nit.
  • Nits I left: the new cluster-user comment cites harperdb line numbers without naming the 4.7.39 tag, which the adjudicator says will rot. fromPermission orders the permission's top-level keys twice per edit, which costs one shallow object per click and keeps a single serializer. PreservedEntries and RoleEditorSchema are exported with no importer, but they type exported functions' results. The remaining comment-narration nit is a style call.
  • Decisions the adjudicator listed: the first slice, blocking Save on notInSchema, Harper 4's cluster user type and a revision precondition are the Your calls above. Leaving unlisted tables out of a new role gives the same effective access, because Harper reads an absent table as no access (permissionsTranslator.js translateRolePermissions). Round 6 added one more: a layout-effect ref for the table grid's updater, versus a functional setDraft(prev => …) updater. I kept the ref, which leaves the draft one write path. A functional updater would add a second one, plus a null branch for a permission the visual editor never sees as null.

  I rejected these on evidence:

  • Gemini said three times that preparePermissionForSave keeps a false flag or drops an array structure_user's table grants. It doesn't: the early return tests === true, the === false branch drops false flags, and the tests asserting both pass at HEAD.
  • Gemini said an absent attribute_permissions is valid. Harper rejects it (role_validation.ts:196-198, ATTR_PERMS_ARRAY_MISSING), so it stays a blocker, now with a message that says how to fix it.
  • Gemini called the per-keystroke parse a stall. It measured 0.1ms for a 78KB role, and the old editor parsed per keystroke too.
  • Codex asked for file extensions on imports. This repo's imports are extensionless @/ aliases.

• gemini-code-assist on the PR raised three render-purity points. Two were real and are fixed. 8ad3f5e stashes the structure-user selection in the mode handler instead of writing a ref during render. 2b87940 refreshes the table grid's updater ref in a layout effect and stops reading the scoping-restore ref during render. I declined the third, on the thread. The guarded setPreferJson during render is React's supported pattern, and the suggested derivation would switch views under the author. 8722738 adds the StrictMode harnesses and the test that pins this.

_{🤖 Generated by Claude Code (Claude Opus); posted via @dawsontoth.}

Related PRs: #1875 overlaps (the base of this stack; this diff deletes the defaultCalculator it changed and keeps its save-side null → [] repair), #1888 overlaps (adds maxLength to the same Add Role name input and creates AddRoleModal.test.tsx against the old props; whichever lands second rebases)
Complexity: complicated

_{Review-Coverage: authored=claude; ran=codex,gemini; adjudicated=domain; blocked=cursor-grok(no-receipt),cursor-composer(no-receipt); declined=cursor-kimi,cursor-muse; rounds=6; full=2 @ 8722738}

_{Review-Attention: study ~20m (sensitive: addRole.ts, addRoleFormSchema.ts; decisions: do-less-alternative, committed-ref-vs-functional-updater, schema-invalid-save-policy, client-side-revision-check, offer-cluster-user-on-v4, single-step-add) @ 8722738}

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive visual role editor for HarperDB, allowing users to configure role types, structure user settings, operations allowlists, and granular table and attribute permissions through an interactive GUI alongside the existing JSON editor. It updates the role creation and editing modals to use this new unified editor. The review feedback highlights several React anti-patterns in the newly added components, specifically recommending against setting state during the render phase in RolePermissionEditor and mutating refs directly during render in RoleVisualEditor, suggesting instead to derive state or defer ref mutations to useLayoutEffect.

[gemini-code-assist]

Setting state during the render phase (setPreferJson(!visualAvailable)) is a React anti-pattern that can lead to extra re-renders or infinite loops. Instead, we can derive preferJson directly from the state and the visual editor's availability. This completely avoids setting state during render and simplifies the component's render logic.

Note: If you apply this suggestion, please also update the state setters on lines 112 and 159 from setPreferJson(...) to setUserPreferJson(...).

	// Undecided until the schema loads; then the view opens on the visual editor when it can.
	const [userPreferJson, setUserPreferJson] = useState<boolean | null>(null);
	// The JSON view re-checks on every keystroke; deferring keeps typing responsive on a large role.
	const deferredPermission = useDeferredValue(permission);

	const visualBlockers = useMemo(() => {
		if (!instanceDatabaseMap || !version || !permission) {
			return ['The JSON is not a valid object.'];
		}
		return getVisualEditorBlockers(permission, instanceDatabaseMap, version);
	}, [permission, instanceDatabaseMap, version]);

	const visualAvailable = schemaReady && visualBlockers.length === 0;
	const preferJson = userPreferJson ?? !visualAvailable;

	const analysed = preferJson ? deferredPermission : permission;
	const blockers = useMemo(() => {
		if (!instanceDatabaseMap || !version) {
			return [];
		}
		return analysed
			? getVisualEditorBlockers(analysed, instanceDatabaseMap, version)
			: ['The JSON is not a valid object.'];
	}, [analysed, instanceDatabaseMap, version]);

[dawsontoth]

Declining this one, on evidence.

The setPreferJson call is React's supported storing information from previous renders pattern: a guarded update to the component's own state during render. preferJson === undefined is true only until the first render that has the schema, so it fires once per mount, and React re-runs that render before committing anything. The guard is what prevents a loop: with it removed, 22 modal tests fail with "Too many re-renders". Under StrictMode's double render both passes compute the same value, and both modal suites now run under StrictMode (8722738).

The suggested derivation changes behavior. userPreferJson ?? !visualAvailable keeps following availability until the author clicks, so a role that opened in the JSON view would jump to the visual editor as soon as the JSON became representable, unmounting Monaco mid-edit. It also runs getVisualEditorBlockers a second time on the non-deferred permission on every keystroke, which undoes the useDeferredValue. A new test pins the first point, and it fails with the suggestion applied.

_{🤖 Generated by Claude Code (Claude Opus); posted via @dawsontoth.}

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	68.4%	10458 / 15289
🔵	Statements	68.57%	11166 / 16284
🔵	Functions	62.01%	2720 / 4386
🔵	Branches	63.73%	8026 / 12593

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/features/instance/config/roles/index.tsx	65.38%	70%	30%	62.5%	37-38, 48, 51-52, 55, 60, 65, 104
src/features/instance/config/roles/preparePermissionForSave.ts	94.73%	96.77%	100%	94.73%	61
src/features/instance/config/roles/editor/RolePermissionEditor.tsx	94.73%	96.77%	88.88%	94.73%	40-41
src/features/instance/config/roles/editor/RoleVisualEditor.tsx	99.11%	94.06%	98.3%	98.96%	281
src/features/instance/config/roles/editor/rolePermissionModel.ts	96.1%	91.32%	98.07%	95.96%	98, 162, 165-166, 180, 274, 283, 294, 309, 616
src/features/instance/config/roles/editor/useRolePermissionDraft.ts	100%	100%	100%	100%
src/features/instance/config/roles/modals/AddRoleModal.tsx	81.25%	85.71%	75%	81.25%	61, 73-74
src/features/instance/config/roles/modals/EditRoleModal.tsx	75%	95.23%	55.55%	73.52%	57, 67-69, 94-103, 144
src/integrations/api/instance/auth/addRole.ts	0%	100%	0%	0%	14-28
src/integrations/api/instance/auth/addRoleFormSchema.ts	100%	100%	100%	100%

Generated in workflow #2162 for commit d9c3134 by the Vitest Coverage Report Action