Skip to content

Manage auth collection accounts in Users - #481

Merged
valthon merged 2 commits into
mainfrom
codex/issue-478-auth-users
Nov 8, 2026
Merged

valthon merged 2 commits into
mainfrom
codex/issue-478-auth-users

Conversation

@valthon

@valthon valthon commented Nov 7, 2026 •

Copy link
Copy Markdown
Owner

Auth collections now have collection-scoped account management at /_/#/users/<collection>, with credentials and custom profile fields in the same form. The sidebar groups collections by type, and ordinary collections keep the generic record editor.

Admin UI

  • Collection-scoped links, reloads and browser history keep the selected collection. Legacy auth record URLs redirect to the same collection's Users page, and collection settings and Users link to each other.
  • Create and edit forms cover credentials and custom profile fields, including required fields, JSON, rich text, relations, multiple selections and file uploads, with accessible shared controls.
  • Editing sends only changed fields and credentials. JSON comparison ignores object-member order but not array order, so concurrent API or hook updates and untouched legacy gaps survive a credential reset.
  • One guarded refresh owns collection metadata, so only the latest request publishes results and a failed lookup does not block later routes. A failed lookup retries when its route is next visited, rather than in a loop, and its error shows only on the route it was for.
  • A select declaring an empty option keeps it distinct from no selection, in JSON and multipart saves alike.
  • Record and account drawers load the current collection definition when opened and block saves while it is loading or failed. Password length uses UTF-8 bytes to match the server, and a zero minimum is preserved.

Record and collection input

  • An empty multipart value clears a single-valued select or relation. A single select that declares "" as an option keeps it as a value and is cleared with null instead, unless null is also an option.
  • Float fields accept integral JSON numbers beyond the i64 range and apply their bounds to them. Number bounds in schema JSON accept the same tokens and refuse strings and non-finite values.
  • The collection API returns a field error unless minPasswordLength is an integer from 0 through 255.

The series is two commits: the input parsing, then the admin UI that depends on it.

Fixes #478.

Validation on Zig 0.16.0, on main at 7e212f30:

  • zig build test --summary all returned 0 at both commits.
  • tests/admin with -n 4: 665 passed, 154 skipped.
  • check-gating, allocator contracts, disabled agent CLI, error-code ledger, doctor ledger and changelog assembly passed.

The zap transport re-pin, its changelog fragment and the tests/admin fixture and diagnostics work that were previously on this branch are removed, as they do not belong to this issue. The prior head is preserved on codex-does-too-much-481.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HTAHrkfe1L2hfpLeP1CQAS

Copilot AI balanced review requested due to automatic review settings November 7, 2026 19:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Account profile saves can silently retain cleared values, alter JSON types, and drop existing relations, while float inputs and unset required booleans can cause validation failures.

4 open issues
What changed in this PR

This PR gives auth collections a single, collection-scoped account workspace in Users, addressing #478 while retaining ordinary record management for data collections.

Changes:

  • Groups navigation by collection type, preserves scoped links, and redirects legacy auth-record URLs.
  • Extends account forms with shared profile-field editors, validation, and file handling.
  • Adds browser regressions and documents the unified workflow.
File Description
tests/​admin/​test_shell.py Updates the built-in account navigation smoke test.
tests/​admin/​test_auth_navigation.py Adds navigation, account-form, and system-protection regressions.
src/​admin/​views/​users.js Adds collection-scoped Users pages and custom profile fields.
src/​admin/​views/​collections.js Shares field controls and serialization; adds Users handoffs.
src/​admin/​lib/​api.js Enables multipart account creation and updates.
src/​admin/​app.js Groups sidebar links and routes auth collections to Users.
docs/​framework.md Documents account navigation and editing capabilities.
changelog.d/​auth-users-navigation.md Records the navigation and account-form changes.

Review effort: Balanced

Comment thread src/admin/views/collections.js Resolved
Comment thread src/admin/views/collections.js Resolved
Comment thread src/admin/views/collections.js Resolved
Comment thread src/admin/views/users.js Resolved
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from b7d81d7 to d8eb887 Compare November 7, 2026 20:35
Copilot AI balanced review requested due to automatic review settings November 7, 2026 20:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Active issues block valid non-ASCII passwords in the user form and prevent file changes from saving when an optional single-select field is unset.

2 open issues
4 resolved since last review

Review effort: Balanced

Comment thread src/admin/views/collections.js Resolved
Comment thread src/admin/views/users.js Resolved
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from d8eb887 to 62831b8 Compare November 7, 2026 21:39
Copilot AI balanced review requested due to automatic review settings November 7, 2026 21:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Clearing an optional single select sends an empty string instead of null when no file changes, causing the server to reject the save.

2 open issues
1 resolved since last review

Review effort: Balanced

Comment thread src/admin/views/collections.js Resolved
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from 62831b8 to c90b756 Compare November 7, 2026 22:10
Copilot AI balanced review requested due to automatic review settings November 7, 2026 22:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved issues misdirect collection user-management links, block account updates for server-accepted internationalized email addresses, and cause inconsistent numeric validation between JSON and multipart submissions.

3 open issues
2 resolved since last review

Review effort: Balanced

Comment thread src/admin/views/collections.js Resolved
Comment thread src/admin/views/collections.js Resolved
Comment thread src/admin/views/users.js Resolved
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from c90b756 to 29d6679 Compare November 7, 2026 23:44
Copilot AI balanced review requested due to automatic review settings November 7, 2026 23:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Cleanup omits rep_setup, so a fixture setup failure while the server is still running can delete server.log without emitting diagnostics.

1 open issue
3 resolved since last review

Review effort: Balanced

Comment thread tests/admin/conftest.py Resolved
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from 29d6679 to 0b525f7 Compare November 8, 2026 00:22
Copilot AI balanced review requested due to automatic review settings November 8, 2026 00:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Profile-field labels are not associated with their controls, leaving required and other inputs unnamed for screen-reader users.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread src/admin/views/users.js Resolved
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from 0b525f7 to c8dd7c9 Compare November 8, 2026 00:57
Copilot AI balanced review requested due to automatic review settings November 8, 2026 00:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved issues can overwrite untouched null rich-text values, block empty passwords permitted by collection policy, and hide unexpected test-server exits.

3 open issues
1 resolved since last review

Review effort: Balanced

Comment thread src/admin/lib/editor.js Resolved
Comment thread src/admin/views/users.js Resolved
Comment thread tests/admin/conftest.py Resolved
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from c8dd7c9 to efd2032 Compare November 8, 2026 01:36
Copilot AI balanced review requested due to automatic review settings November 8, 2026 01:36
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from e29f12c to e37f457 Compare November 8, 2026 05:35
Copilot AI balanced review requested due to automatic review settings November 8, 2026 05:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues block approval of the collection-scoped account-management changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@valthon valthon left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This re-review covers e37f4570. I reviewed by reading the code; I did not run it.

All seven issues from the previous review are addressed: changed-fields-only PATCH, no hang on '…', rich-text cleanup on blur, blank username omitted, a shared system-field helper, the zero password minimum, and reuse of the Shell's metadata. Thanks.

Changes requested (blocking). GitHub doesn't allow a formal "Request changes" on your own PR, so this review is a comment, but every item below must be resolved before merge:

  1. Regression: the record drawer hides email, username, verified and token_epoch on base collections. This breaks creating and editing _suppressions and _sender_identities records.
  2. Correctness: one failed cache-miss refresh leaves err set, blocking later refreshes and showing a false error on every records page.
  3. Required refactor: replace the Shell's metadata-refresh state machine with a single refreshCollections() (details inline). Item 2 comes from its complexity, and we don't want to keep that much interdependent state in the admin shell.
  4. Correctness: RecordsTable now reads an outdated schema snapshot, so fields added elsewhere are missing from the drawer until a reload.
  5. Correctness: minPasswordLength accepts out-of-range or non-integer values that the server silently drops.

Please add browser regressions for 1, 2 and 4. As CLAUDE.md notes, zig build test won't catch them.


Generated by Claude Code

Comment thread src/admin/views/collections.js Resolved
Comment thread src/admin/app.js Resolved
Comment thread src/admin/app.js Resolved
Comment thread src/admin/views/collections.js Resolved
Comment thread src/admin/views/collections.js Resolved
Copilot AI balanced review requested due to automatic review settings November 8, 2026 13:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The boundary-case tests use an arena that masks potential allocation leaks in parseCollectionInput, leaving the required raw testing-allocator coverage for owned results missing.

1 open issue

Review effort: Balanced

Comment thread src/schema.zig Resolved
Copilot AI balanced review requested due to automatic review settings November 8, 2026 14:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because an unpublished finding indicates that cached metadata lookup failures can keep New record disabled even after the API recovers.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@valthon
valthon force-pushed the codex/issue-478-auth-users branch from e11ca19 to 37abd0d Compare November 8, 2026 20:08
Copilot AI balanced review requested due to automatic review settings November 8, 2026 20:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Single-select handling conflates empty-string options with null during saves, risking incorrect stored values, while navigation to cached auth collections leaves stale lookup errors visible.

3 open issues

Review effort: Balanced

Comment thread src/admin/app.js Resolved
Comment thread src/admin/views/collections.js Resolved
Comment thread src/admin/views/collections.js Resolved
Forms that submit a whole record exposed gaps in how the API accepts
input. Clearing a single-valued select or relation in a multipart
update sent an empty string, which was validated as a literal option
or target id and rejected. Integral JSON numbers outside the i64 range
arrive as number strings, so float fields skipped their bounds check
and failed to bind, and such bounds could not round-trip through schema
JSON. An invalid minPasswordLength fell back silently.

An empty multipart value now clears single-valued selects and relations
as it already did for multi-valued ones. A single select declaring ""
as an option keeps it as a value, so that select is cleared by the JSON
spelling null instead, unless null is an option too.

values.floatValue converts typed JSON numbers to a finite float for
both validation and binding, so bounds apply to large tokens and
non-finite values are refused while ordinary strings keep their type
error. Number bounds in schema JSON
accept the same tokens and refuse strings and non-finite values.

The collection create and update endpoints return a field error unless
minPasswordLength is an integer from 0 through 255, instead of a
generic bad request or a fallback.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HTAHrkfe1L2hfpLeP1CQAS
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from 37abd0d to dccea21 Compare November 8, 2026 20:47
Copilot AI balanced review requested due to automatic review settings November 8, 2026 20:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A delayed lookup failure can incorrectly appear on the members page and sidebar after navigation, requiring a current-route guard and a delayed-failure regression test.

1 open issue
3 resolved since last review

Review effort: Balanced

Comment thread src/admin/app.js Resolved
Auth collections sat beside ordinary collections in the admin sidebar
and opened the generic Records screen, whose New record form could not
collect a password. The Users editor handled credentials but omitted
custom profile fields, and it chose its own default collection, so a
link could not reliably keep the collection in view.

The sidebar now groups collections by type: ordinary ones under Data
collections and auth ones by name under Users. Each opens account
management at /_/#/users/<collection>, kept across reloads and browser
history, and legacy auth record URLs redirect to the same collection.
Collection settings and Users link to each other, and _superusers keeps
its system protections.

Create and edit forms cover credentials together with custom profile
fields, including required fields, JSON, rich text, relations, multiple
selections and file uploads, using shared accessible controls. Editing
sends only changed fields and credentials, so concurrent API or hook
updates and untouched legacy gaps survive a credential reset. JSON
comparison ignores object-member order but not array order. Valid field
names such as constructor stay ordinary keys.

Collection metadata has one guarded refresh, so only the latest request
publishes results and a failed lookup does not block later routes. It
retries when the route is next visited rather than in a loop, and its
error shows only on the route it was for. Record and account drawers
load the
current collection definition when opened and block saves while it is
loading or failed, so field and password-policy changes made elsewhere
apply immediately. Password length uses UTF-8 bytes to match the
server, and a zero minimum is preserved. A select declaring an empty
option keeps it distinct from no selection, in JSON and multipart saves
alike. Base collections keep their ordinary email and username fields,
and schema controls protect system fields by persisted identity rather
than by a draft name.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HTAHrkfe1L2hfpLeP1CQAS
@valthon
valthon force-pushed the codex/issue-478-auth-users branch from dccea21 to 08fe0f5 Compare November 8, 2026 22:29
Copilot AI balanced review requested due to automatic review settings November 8, 2026 22:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

There are no active review issues for the collection-scoped account management changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@valthon
valthon merged commit d0853a9 into main Nov 8, 2026
31 checks passed
@valthon
valthon deleted the codex/issue-478-auth-users branch November 8, 2026 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature] Separate auth collections into Users navigation in admin

2 participants