Repository navigation
feat: user directory protocol; modules stop importing users.models; strict dependency discovery (#501) - #566
Merged
Merged
Conversation
Core protocols so modules read users and recent activity without importing a provider module: DirectoryUser/DirectoryStats, the UserDirectory protocol with a NullUserDirectory fallback, the shared naming rule (DirectoryUser.label), id parsing/chunking helpers, and the ActivityFeed protocol, each registered on app.state. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
UsersDirectory over users_user (roles via users_user_role, disabled accounts inactive); users.audit's label resolver delegates to it. KeycloakDirectory over keycloak_user_cache: users who have signed in, roles unknown, no creation date; match_ids renders hyphenated ids on SQLite where Uuid is bare hex. Each provider registers its directory in register_settings. users.contracts.roles re-exports Role for the permissions role pages. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…ty feed breaks the cycle (#501) audit_log no longer imports users: actors are named and searched through the app's user directory (actor_filter stays an unbounded SQL subquery via match_ids, and falls back to an exact id match without one), actor links key off the directory's entity_type, and depends_on=[Users] is gone. audit_log registers an ActivityFeed; the users edit page's Recent activity card reads it instead of lazily importing audit_log, keeping 'no feed' (no card) distinct from 'no activity' (empty card). Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
… onto the user directory (#501) - dashboard: user tiles from UserDirectory.stats(); a None figure hides its tile; Users dependency (meta and package) dropped. - file_storage: uploader names via the directory; the copied users query is gone. - api_tokens: owner checks and emails via get_many; PAT resolution via get_many(with_roles=True), refusing owners whose roles are unknown. Keeps depends_on Users. - permissions: user side via get_many/search; role pages keep Users through users.contracts.roles; admin sync uses core ADMIN_ROLE. - auth: a structural AccountLike protocol replaces the users.User hint. - background_tasks: drop the unused Users dependency. - smpy catalog: requires now mirrors depends_on (users needs settings, background_tasks needs settings, dashboard needs nothing), pinned by a test. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
New docs/framework/user-directory.md (sidebar entry), module docs for audit_log, dashboard, background_tasks, api_tokens, permissions, users and keycloak, a CLAUDE.md architecture note, the conventions skill rule, and CHANGELOG entries. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
topological_sort(modules, strict=...) collects every (module, dependency) pair whose dependency is not loaded; strict raises one MissingDependencyError (an InvalidModuleError) naming them all. create_app passes strict=not is_development; development keeps logging per pair and the new SM030 diagnostic reports it (warning at a dev boot, error in make doctor). The outbox worker stays lenient. ModuleMeta.optional_depends_on orders after a module when it is loaded and is never missing. The ordering-only Settings entries (users, tenants, file_storage, background_tasks, site_lock, api_tokens, keycloak) move there, keeping the boot order unchanged and the settings plugin optional (#500). The smpy catalog no longer makes users require settings. Keycloak test apps leave out modules that need Users (permissions, api_tokens) instead of booting leniently; a test pins that the full set fails a non-development boot naming both. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…owner check, trim stale docstrings Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Deploying simple-module-python with
|
| Latest commit: |
cb480e6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3bf1e935.simple-module-python.pages.dev |
| Branch Preview URL: | https://feat-user-directory-501.simple-module-python.pages.dev |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 task
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.
Summary
Closes #501.
On a Keycloak install, modules that imported
users.modelsdirectly were quietly querying emptyusers_*tables. They now go through a framework user directory, which the installed auth provider supplies.simple_module_core.user_directory):UserDirectoryprotocol:get_many(batched; keyed by the caller's id strings; invalid or unknown ids are left out),search,match_ids(a SELECT, so the audit actor filter stays an unbounded subquery) andstats.DirectoryUser, whoselabelis the one naming rule: full name, else email.NullUserDirectorywhen no provider is installed.simple_module_core.activity_feed):ActivityFeedbreaks theusers↔audit_logimport cycle.audit_logregisters it; the users "recent activity" card reads it, with the permission gating unchanged (GH Audit log entity labels expose account names and emails toaudit_log.viewholders #300).users/directory.py, andkeycloak/directory.pyover its user cache table. Under Keycloak, counts mean "users who have signed in at least once", "created this month" is unknown, and roles are unknown.audit_log: actor names and actor filter;dashboard: user tiles (a tile with an unknown value is hidden);file_storage: its copy of the label resolver is deleted;api_tokens: owner checks and personal-token resolution;permissions: user lookup and search;auth: a structuralAccountLiketype replaces theusers.Userhint;background_tasks: unused dependency dropped.depends_ontarget is missing refuses the boot with oneMissingDependencyErrorlisting every missing pair.make doctorreports it as SM030.ModuleMeta.optional_depends_on: sorts after a module when it is present and never requires it. It's used for modules that listedSettingsonly for load order, so the app still boots withsettingsdisabled (Design: thesettingsplugin module has become de facto framework; make the dependency explicit #500), and for ApiTokens → Users.Decisions
Users, because its role pages need theRoletable. A strict Keycloak boot with Permissions enabled therefore fails with a clear error instead of running against empty tables. Keying roles by name for Keycloak is a follow-up.Verification
make lintpasses.make test-py: 5069 passed, 22 skipped.make test-js: 668 passed.make ci-doctor: 0 errors./ship:Test plan
make doctorhttps://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4