Skip to content

refactor(authnz): centralize membership writes through ordered protocol - #2821

Open
rmusser01 wants to merge 23 commits into
devfrom
codex/userprofiles-stage2-membership-writer
Open

rmusser01 wants to merge 23 commits into
devfrom
codex/userprofiles-stage2-membership-writer

Conversation

@rmusser01

@rmusser01 rmusser01 commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Change summary\n\n_Human-authored summary required before merge._\n\n## Technical summary\n\n- Introduce the immutable membership-write protocol, deterministic lock planning, and transaction-aware execution.\n- Route direct membership, ownership, provisioning, default-team, scope-deletion, and shared-key authorization paths through managed writers.\n- Preserve existing public contracts while adding bounded acquisition, persisted authorization rechecks, profile-version anchoring, concurrency protection, and sanitized retry responses.\n\n## Verification\n\n- 289 affected service, repository, endpoint, and structural tests passed.\n- 15 SQLite membership and scope-deletion tests passed.\n- 47 live PostgreSQL registration, BYOK, locking, and concurrency tests passed.\n- Ruff, compileall, and git diff checks passed.\n- Bandit medium/high scan reported zero findings; no new all-severity finding was introduced.\n\n## Tracking\n\n- Backlog: TASK-13001.2\n


Summary by cubic

Centralizes all org/team membership mutations and reads behind an ordered, transaction-aware MembershipWriter and forbids raw writes (including COPY) to membership tables. This reduces deadlocks and write skew, preserves existing APIs, and returns retryable 503s when the auth DB is busy. Addresses TASK-13001.2.

New Features

  • Routes invites, org bootstrap/ownership with default-team, shared-key auth, federation provisioning, BYOK secrets, and scope deletion through the managed writer.
  • Plans deterministic cross-user/scope locks, anchors to profile versions, and rechecks authorization before duplicate checks inside the transaction.
  • Persists membership authority (platform admin vs scoped) and requires endpoints/services to pass the correct authority.
  • Adds bounded transaction acquisition and maps pool exhaustion/locks to sanitized 503s across auth, invites, BYOK, and registration flows.
  • Blocks direct INSERT/UPDATE/DELETE/COPY on org_members and team_members and repairs their timestamps via managed paths.
  • Locks user rows before role revocation on Postgres and adds guarded savepoints/exec paths for SQLite.

Migration

  • Do not write to org_members/team_members directly; call the MembershipWriter via repos/services with an ActorMembershipWriteContext or TrustedMembershipWriteContext.
  • Platform-admin routes must pass platform authority; scoped routes must pass scoped membership authority.
  • Clients should retry on 503s when the auth database is busy.

Written for commit 9f175b2. Summary will update on new commits.

Review in cubic

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 283ca8f0-3f36-4f07-a0bc-0d3419e33145

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Centralize membership writes via ordered, transaction-aware MembershipWriter

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Centralize org/team membership writes through deterministic, transaction-aware MembershipWriter.
• Route provisioning, invites, admin, and shared-key flows through managed writers.
• Harden DB safety with write-guards, retryable 503s, and broad concurrency tests.
Diagram

graph TD
A["API endpoints"] --> B["AuthNZ services"] --> C["Orgs/Teams repo"] --> D["MembershipWriter"] --> E["Write guard"] --> F[("AuthNZ DB")]
D --> G["ProfileVersion gateway"] --> F
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. DB advisory locks only
  • ➕ Less application-side lock planning logic
  • ➕ Can avoid some row-lock ordering complexity
  • ➖ Harder to prove correctness across scope/user/owner rows
  • ➖ Advisory locks are DB-specific and less portable to SQLite
  • ➖ Still needs in-transaction authorization rechecks and version anchoring
2. Serializable isolation / retry loop
  • ➕ Pushes concurrency correctness to the database engine
  • ➕ Simplifies some lock ordering requirements
  • ➖ Potentially higher contention/latency under load
  • ➖ Error handling becomes broader and more opaque
  • ➖ Still needs deterministic conflict surfacing and sanitization for clients
3. Event-driven membership writes (queue)
  • ➕ Avoids synchronous lock contention in request path
  • ➕ Can batch and order mutations centrally
  • ➖ Changes API semantics (eventual consistency)
  • ➖ Operational complexity (queue, retries, idempotency, DLQ)
  • ➖ Not aligned with “preserve existing public contracts” goal

Recommendation: The PR’s approach (immutable write protocol + deterministic lock planning + in-transaction authorization rechecks + profile version floors) is the best fit because it preserves existing API contracts while materially reducing deadlock/race risk. Advisory locks or serializable isolation could reduce code, but would either sacrifice portability/clarity or broaden retry behavior; the explicit lock plan and sanitized 503 retry surface here is more predictable for clients and easier to validate via the added concurrency tests.

Files changed (57) +15099 / -1479

Enhancement (9) +3767 / -180
admin_tenant_provisioning.pyRoute tenant provisioning membership creation via managed writer +72/-16

Route tenant provisioning membership creation via managed writer

• Moves initial tenant org membership creation through AuthnzOrgsTeamsRepo/MembershipWriter, adds bounded transaction acquisition, and maps membership/lock exhaustion to sanitized 403/503 HTTP errors.

tldw_Server_API/app/api/v1/endpoints/admin/admin_tenant_provisioning.py

orgs.pyCentralize membership-control error mapping and context derivation +167/-33

Centralize membership-control error mapping and context derivation

• Adds helpers to build ActorMembershipWriteContext (scoped vs platform-admin) and to map membership-writer/DB-busy failures into consistent 403/404/409/503 responses with Retry-After.

tldw_Server_API/app/api/v1/endpoints/orgs.py

database.pyExtend write guard and add SQLite savepoint helpers +21/-1

Extend write guard and add SQLite savepoint helpers

• Blocks COPY to org_members/team_members in Postgres guard paths and adds validated SQLite savepoint helpers (create/rollback/release) for transaction-aware writer behavior.

tldw_Server_API/app/core/AuthNZ/database.py

provisioning_service.pyApply federated grants via membership writer with bounded transactions +88/-24

Apply federated grants via membership writer with bounded transactions

• Routes federated org/team grant application through repo methods that perform managed membership writes inside bounded transactions, and sanitizes logging to avoid leaking exception details.

tldw_Server_API/app/core/AuthNZ/federation/provisioning_service.py

membership_writer.pyIntroduce immutable membership-write protocol and ordered lock execution +2620/-0

Introduce immutable membership-write protocol and ordered lock execution

• Adds the MembershipWriter protocol: closed contracts for mutations/contexts, deterministic lock-set planning (users/scopes/membership/owner rows), in-transaction preflight rechecks and persisted authorization checks, and version-floor anchoring via ProfileVersion gateway integration. Provides sanitized error types for callers and supports Postgres vs SQLite execution differences.

tldw_Server_API/app/core/AuthNZ/membership_writer.py

org_provider_secrets_repo.pyLock and authorize shared BYOK scope writes using persisted membership +427/-26

Lock and authorize shared BYOK scope writes using persisted membership

• Adds parent-scope locking and authorization checks (scoped manager vs platform admin) and threads ActorMembershipWriteContext through secret mutation/list/fetch operations with bounded acquisition semantics.

tldw_Server_API/app/core/AuthNZ/repos/org_provider_secrets_repo.py

service.pyAdd canonical multi-user profile lock helper +20/-0

Add canonical multi-user profile lock helper

• Introduces lock_profile_users(user_ids, db_conn) to lock multiple profile users in sorted order using the profile version gateway.

tldw_Server_API/app/core/UserProfiles/service.py

update_service.pyBatch membership mutations via MembershipWriter during profile updates +255/-79

Batch membership mutations via MembershipWriter during profile updates

• Prepares membership mutations from profile update keys, applies them as a batch through MembershipWriter with consistent operation timestamps, and folds version floors into the caller-owned profile anchor to keep reads/writes transaction-consistent.

tldw_Server_API/app/core/UserProfiles/update_service.py

admin_byok_service.pyAuthorize shared BYOK key operations with membership contexts and retryable 503s +97/-1

Authorize shared BYOK key operations with membership contexts and retryable 503s

• Introduces platform-admin membership write contexts for shared key operations, authorizes scope writes via repo methods, and maps DB-busy conditions to sanitized 503 Retry-After responses.

tldw_Server_API/app/services/admin_byok_service.py

Bug fix (11) +635 / -222
org_invites.pyReturn retryable 503 when membership redemption hits DB contention +25/-7

Return retryable 503 when membership redemption hits DB contention

• Wraps invite redemption to translate connection-pool exhaustion / lock timeouts into a sanitized 503 with Retry-After header.

tldw_Server_API/app/api/v1/endpoints/org_invites.py

shared_keys_scoped.pyEnforce persisted membership authority for scoped shared-key operations +121/-28

Enforce persisted membership authority for scoped shared-key operations

• Passes ActorMembershipWriteContext into shared-key repo methods, tightens manager checks to require active memberships, and maps authorization/scope-not-found to 403/404.

tldw_Server_API/app/api/v1/endpoints/shared_keys_scoped.py

byok_runtime.pyHarden BYOK secret resolution and local lock directory creation +20/-47

Harden BYOK secret resolution and local lock directory creation

• Ensures OAuth refresh lock directory creation failures surface as sanitized credential-store errors, and simplifies shared-secret lookup to always use the authorized path (removing an “active-only” bypass).

tldw_Server_API/app/core/AuthNZ/byok_runtime.py

profile_candidate_schema.pyRepair membership timestamps via guarded membership repair helper +15/-5

Repair membership timestamps via guarded membership repair helper

• Routes Postgres org_members/team_members timestamp repairs through the membership-specific repair helper to ensure guarded execution semantics.

tldw_Server_API/app/core/AuthNZ/profile_candidate_schema.py

profile_user_write_guard.pyReject raw membership/scope writes; add membership-scope SQL capability +279/-19

Reject raw membership/scope writes; add membership-scope SQL capability

• Extends SQL classification with domains, introduces a writer-owned membership-scope SQL capability, rejects direct INSERT/UPDATE/DELETE/COPY against membership and parent scope tables, and adds Postgres asyncpg savepoint pattern handling plus timestamp repair helpers.

tldw_Server_API/app/core/AuthNZ/profile_user_write_guard.py

users_repo.pyLock user row before mutating role membership +12/-8

Lock user row before mutating role membership

• Adds an explicit user row lock in Postgres before deleting user_roles to align with deterministic lock ordering and reduce race conditions.

tldw_Server_API/app/core/AuthNZ/repos/users_repo.py

user_provider_secrets.pyMake provider-alias conflict a sanitized registration exception +2/-1

Make provider-alias conflict a sanitized registration exception

• Extends ProviderCredentialAliasConflictError to also be a UserRegistrationException so it can be safely surfaced/mapped like other sanitized AuthNZ errors.

tldw_Server_API/app/core/AuthNZ/user_provider_secrets.py

command_service.pyLock profile users for membership updates and map authorization failures +49/-23

Lock profile users for membership updates and map authorization failures

• Locks actor/target profile rows when membership updates are present and maps MembershipAuthorizationError into a 403-style command result with per-key forbidden skips.

tldw_Server_API/app/core/UserProfiles/command_service.py

admin_profiles_service.pyLock actor/target profile rows for bulk membership updates +27/-6

Lock actor/target profile rows for bulk membership updates

• Detects membership updates in bulk profile updates and locks both actor and target profile users within bounded transactions to ensure concurrency-safe membership modifications.

tldw_Server_API/app/services/admin_profiles_service.py

org_invite_service.pyProvision org + default/team memberships atomically on invite redemption +43/-13

Provision org + default/team memberships atomically on invite redemption

• Replaces ad-hoc org/team membership inserts with a single repo provisioning call within a bounded transaction, preserving best-effort semantics for optional team membership.

tldw_Server_API/app/services/org_invite_service.py

registration_service.pyProvision initial org/team membership via managed repo writer on registration +42/-65

Provision initial org/team membership via managed repo writer on registration

• Replaces direct org_members/team_members SQL writes with repo provisioning using TrustedMembershipWriteContext, adds bounded transaction acquisition, and sanitizes registration failure logging.

tldw_Server_API/app/services/registration_service.py

Refactor (5) +1420 / -731
auth.pyAlign auth bootstrap membership path with managed membership writes +76/-25

Align auth bootstrap membership path with managed membership writes

• Refactors the bootstrap org membership helper to use repo-based membership provisioning with trusted write context, and imports shared transaction policy/utilities to support bounded acquisition and observability.

tldw_Server_API/app/api/v1/endpoints/auth.py

orgs_teams.pyExpose membership APIs that require explicit write contexts +138/-12

Expose membership APIs that require explicit write contexts

• Adds context-aware wrappers (e.g., create_organization_with_owner_membership, create_organization_as_actor, add/remove/update membership methods) to ensure all membership mutations go through managed writer pathways.

tldw_Server_API/app/core/AuthNZ/orgs_teams.py

orgs_teams_repo.pyMigrate membership and scope-deletion mutations into ordered writer flows +1051/-666

Migrate membership and scope-deletion mutations into ordered writer flows

• Integrates MembershipWriter across org/team membership operations (including provisioning and scope deletion), adds bounded transaction acquisition helpers, and propagates version floors/anchor final-touches to prevent write skew under concurrency.

tldw_Server_API/app/core/AuthNZ/repos/orgs_teams_repo.py

admin_e2e_support_service.pySeed E2E org memberships through managed membership contexts +24/-8

Seed E2E org memberships through managed membership contexts

• Switches org creation to the repo’s create_organization_with_owner_membership and passes trusted membership write context for subsequent member additions to respect writer boundaries.

tldw_Server_API/app/services/admin_e2e_support_service.py

admin_orgs_service.pyUse actor/scoped membership contexts and consistent error mapping for admin org ops +131/-20

Use actor/scoped membership contexts and consistent error mapping for admin org ops

• Builds ActorMembershipWriteContext from principal scope, routes org creation through actor-aware org creation helpers, and maps membership control failures (authorization/preflight/scope/target) to appropriate HTTP errors.

tldw_Server_API/app/services/admin_orgs_service.py

Tests (30) +9153 / -344
test_admin_byok_service_sanitizers.pyExpand sanitization coverage for BYOK shared-key membership errors +255/-1

Expand sanitization coverage for BYOK shared-key membership errors

• Adds/updates tests to ensure shared BYOK admin endpoints map membership/lock/pool errors into the expected sanitized HTTP responses.

tldw_Server_API/tests/Admin/test_admin_byok_service_sanitizers.py

test_auth_principal_api_key_happy_path.pyUpdate integration expectations for auth principal flow changes +11/-5

Update integration expectations for auth principal flow changes

• Adjusts integration assertions to match updated auth/membership behavior and error mapping.

tldw_Server_API/tests/AuthNZ/integration/test_auth_principal_api_key_happy_path.py

test_authnz_orgs_teams_repo_postgres.pyAlign Postgres org/team repo integration tests with writer-based mutations +88/-52

Align Postgres org/team repo integration tests with writer-based mutations

• Updates integration tests to exercise the new membership writer-backed repo behavior and any changed return/error semantics.

tldw_Server_API/tests/AuthNZ/integration/test_authnz_orgs_teams_repo_postgres.py

test_registration_role_membership_postgres.pyValidate registration membership provisioning through MembershipWriter on Postgres +160/-69

Validate registration membership provisioning through MembershipWriter on Postgres

• Updates registration integration tests to reflect writer-based provisioning and verifies correct membership/role outcomes under Postgres.

tldw_Server_API/tests/AuthNZ/integration/test_registration_role_membership_postgres.py

test_admin_tenant_provisioning.pyCover tenant provisioning via managed membership writer and busy retries +297/-22

Cover tenant provisioning via managed membership writer and busy retries

• Extends tenant provisioning tests to validate writer-based org membership creation and sanitized busy/authorization responses.

tldw_Server_API/tests/AuthNZ/test_admin_tenant_provisioning.py

test_admin_orgs_service_backend_selection.pyUpdate admin orgs service unit tests for new membership context routing +238/-1

Update admin orgs service unit tests for new membership context routing

• Adjusts backend-selection tests to reflect refactored org creation paths and membership-context based behavior.

tldw_Server_API/tests/AuthNZ/unit/test_admin_orgs_service_backend_selection.py

test_admin_rbac_error_mapping.pyAdd unit tests for RBAC/membership error-to-HTTP mapping +62/-0

Add unit tests for RBAC/membership error-to-HTTP mapping

• Introduces tests ensuring membership authorization/conflict/scope-not-found map to consistent HTTP-level responses.

tldw_Server_API/tests/AuthNZ/unit/test_admin_rbac_error_mapping.py

test_auth_endpoints_extended.pyAlign auth endpoint unit tests with bounded acquisition and membership bootstrap +37/-6

Align auth endpoint unit tests with bounded acquisition and membership bootstrap

• Updates endpoint tests to match new transaction policy usage and membership bootstrap behavior.

tldw_Server_API/tests/AuthNZ/unit/test_auth_endpoints_extended.py

test_auth_register_strict_audit.pyAdd strict audit expectations for registration path with writer provisioning +68/-0

Add strict audit expectations for registration path with writer provisioning

• Adds tests ensuring registration auditing remains correct after moving membership writes behind the writer.

tldw_Server_API/tests/AuthNZ/unit/test_auth_register_strict_audit.py

test_authnz_orgs_teams_repo_backend_selection.pyExpand backend-selection tests for writer-backed repo operations +1062/-38

Expand backend-selection tests for writer-backed repo operations

• Updates/extends unit coverage to ensure SQLite/Postgres repo implementations select correct writer/guarded behaviors.

tldw_Server_API/tests/AuthNZ/unit/test_authnz_orgs_teams_repo_backend_selection.py

test_membership_writer_context.pyAdd unit tests for membership write contexts and contract validation +408/-0

Add unit tests for membership write contexts and contract validation

• Introduces comprehensive tests for Actor/Trusted contexts, authority requirements, and serving/offline contract enforcement.

tldw_Server_API/tests/AuthNZ/unit/test_membership_writer_context.py

test_membership_writer_execution.pyAdd unit tests for membership writer mutation execution semantics +1467/-0

Add unit tests for membership writer mutation execution semantics

• Adds extensive tests covering mutation application, authorization rechecks, preflight change detection, version floors, and error sanitization paths.

tldw_Server_API/tests/AuthNZ/unit/test_membership_writer_execution.py

test_membership_writer_lock_plan.pyAdd unit tests for deterministic lock planning and statement ordering +1148/-0

Add unit tests for deterministic lock planning and statement ordering

• Validates lock-set derivation and Postgres lock statement planning across users/scopes/membership/owner/authority rows.

tldw_Server_API/tests/AuthNZ/unit/test_membership_writer_lock_plan.py

test_org_provider_secrets_repo_row_normalization.pyAdd unit tests for BYOK repo row normalization and authorization locks +434/-0

Add unit tests for BYOK repo row normalization and authorization locks

• Adds coverage for shared secret selection/normalization plus the new scope locking and authorization behavior.

tldw_Server_API/tests/AuthNZ/unit/test_org_provider_secrets_repo_row_normalization.py

test_orgs_endpoint_sanitization.pyUpdate org endpoint sanitization tests for writer error mapping +475/-2

Update org endpoint sanitization tests for writer error mapping

• Adjusts tests to ensure membership control endpoints return sanitized 403/404/409/503 responses as designed.

tldw_Server_API/tests/AuthNZ/unit/test_orgs_endpoint_sanitization.py

test_profile_candidate_schema.pyAdd/adjust tests for membership timestamp repair paths +46/-0

Add/adjust tests for membership timestamp repair paths

• Adds tests ensuring membership table timestamp repairs route through the guarded membership repair helper.

tldw_Server_API/tests/AuthNZ/unit/test_profile_candidate_schema.py

test_profile_user_write_guard.pyAdd tests rejecting raw membership/scope writes and capability misuse +104/-0

Add tests rejecting raw membership/scope writes and capability misuse

• Extends guard tests to ensure direct membership/scope DML and COPY are rejected and membership-scope capabilities are connection-bound and single-use.

tldw_Server_API/tests/AuthNZ/unit/test_profile_user_write_guard.py

test_profile_user_write_managed_boundaries.pyAdd tests for managed membership/profile write boundary enforcement +71/-0

Add tests for managed membership/profile write boundary enforcement

• Adds unit coverage verifying membership writes remain within managed writer/guard boundaries.

tldw_Server_API/tests/AuthNZ/unit/test_profile_user_write_managed_boundaries.py

test_registration_default_role_membership.pyUpdate unit tests for default role membership provisioning +158/-3

Update unit tests for default role membership provisioning

• Aligns unit tests with repo-driven membership provisioning and any updated idempotency semantics.

tldw_Server_API/tests/AuthNZ/unit/test_registration_default_role_membership.py

test_registration_service_backend_selection.pyUpdate backend-selection tests for registration bounded transactions +53/-1

Update backend-selection tests for registration bounded transactions

• Adjusts unit tests to reflect transaction acquire timeout configuration and repo provisioning usage.

tldw_Server_API/tests/AuthNZ/unit/test_registration_service_backend_selection.py

test_users_repo_bootstrap_backend_selection.pyAdd coverage for users repo bootstrap behavior with new locks +33/-0

Add coverage for users repo bootstrap behavior with new locks

• Introduces/extends tests to validate bootstrap paths align with new locking behavior in users repo.

tldw_Server_API/tests/AuthNZ/unit/test_users_repo_bootstrap_backend_selection.py

test_identity_provider_admin_api.pyUpdate federation admin API tests for writer-backed grant application +12/-6

Update federation admin API tests for writer-backed grant application

• Aligns federation admin API tests to reflect managed membership grant application flows.

tldw_Server_API/tests/AuthNZ_Federation/test_identity_provider_admin_api.py

test_oidc_login_flow.pyUpdate OIDC login tests for membership provisioning changes +10/-4

Update OIDC login tests for membership provisioning changes

• Adjusts OIDC flow tests to match updated membership provisioning behavior.

tldw_Server_API/tests/AuthNZ_Federation/test_oidc_login_flow.py

test_provisioning_service.pyUpdate federation provisioning tests for bounded transaction and writer usage +203/-2

Update federation provisioning tests for bounded transaction and writer usage

• Validates that mapped grants use bounded transactions and writer-backed membership operations without leaking raw failures.

tldw_Server_API/tests/AuthNZ_Federation/test_provisioning_service.py

test_admin_endpoints_pg.pyUpdate Postgres admin endpoint tests for membership writer busy/authorization mapping +26/-7

Update Postgres admin endpoint tests for membership writer busy/authorization mapping

• Adjusts Postgres admin endpoint tests to reflect new membership writer behavior and sanitized retryable errors.

tldw_Server_API/tests/AuthNZ_Postgres/test_admin_endpoints_pg.py

test_byok_oauth_endpoints_pg.pyExpand Postgres BYOK tests for authorization contexts and busy retries +244/-112

Expand Postgres BYOK tests for authorization contexts and busy retries

• Updates BYOK OAuth endpoint tests to cover authorization-context enforcement and retryable 503 behavior under contention.

tldw_Server_API/tests/AuthNZ_Postgres/test_byok_oauth_endpoints_pg.py

test_membership_scope_delete_concurrency_pg.pyAdd Postgres concurrency tests for scope deletion under contention +1030/-0

Add Postgres concurrency tests for scope deletion under contention

• Introduces high-contention tests validating scope deletion retries/ordering correctness and absence of deadlocks/races.

tldw_Server_API/tests/AuthNZ_Postgres/test_membership_scope_delete_concurrency_pg.py

test_membership_writer_concurrency_pg.pyAdd Postgres concurrency tests for MembershipWriter lock ordering +872/-0

Add Postgres concurrency tests for MembershipWriter lock ordering

• Adds concurrent mutation tests validating deterministic lock acquisition, preflight change detection, and correct error surfaces.

tldw_Server_API/tests/AuthNZ_Postgres/test_membership_writer_concurrency_pg.py

test_orgs_teams_pg.pyUpdate Postgres org/team tests for repo/writer membership behavior +37/-7

Update Postgres org/team tests for repo/writer membership behavior

• Aligns Postgres org/team tests with writer-based membership mutation semantics.

tldw_Server_API/tests/AuthNZ_Postgres/test_orgs_teams_pg.py

test_profile_candidate_and_tenant_pg.pyUpdate Postgres profile/tenant tests for membership timestamp repair and provisioning +44/-6

Update Postgres profile/tenant tests for membership timestamp repair and provisioning

• Adjusts Postgres tests to validate profile candidate repairs and tenant provisioning behaviors after writer migration.

tldw_Server_API/tests/AuthNZ_Postgres/test_profile_candidate_and_tenant_pg.py

Documentation (2) +124 / -2
task-13001.1 - Work-Package-1-Storage-and-Transaction-Foundations.mdMark WP1 complete and record merge closeout +4/-2

Mark WP1 complete and record merge closeout

• Updates the work-package status to Done and records the merge commit and closeout notes for WP1.

backlog/completed/task-13001.1 - Work-Package-1-Storage-and-Transaction-Foundations.md

task-13001.2 - Work-Package-2-Membership-Writer-Protocol.mdAdd WP2 task plan for membership writer protocol +120/-0

Add WP2 task plan for membership writer protocol

• Introduces the TASK-13001.2 backlog document, including scope, references, and the modified-files inventory for the membership writer rollout.

backlog/tasks/task-13001.2 - Work-Package-2-Membership-Writer-Protocol.md

@qodo-code-review

qodo-code-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Busy failures become partial success ✓ Resolved 🐞 Bug ☼ Reliability
Description
The explicit-team best-effort branch catches every exception from the combined membership write,
including database lock and timeout failures, then retries only the organization/default-team
mutations and allows the outer invite transaction to commit. This bypasses the invite endpoint's
retryable 503 handling and can report successful redemption with the requested explicit-team
membership missing.
Code

tldw_Server_API/app/core/AuthNZ/repos/orgs_teams_repo.py[1735]

+                except Exception:
Evidence
The fallback's broad catch surrounds the full writer operation, whose lock and SQL phases can raise
database infrastructure exceptions. The invite service and endpoint explicitly rely on those
exceptions escaping to produce a 503, but the fallback rolls back only its savepoint and reapplies
the base mutations instead.

tldw_Server_API/app/core/AuthNZ/repos/orgs_teams_repo.py[1711-1727]
tldw_Server_API/app/core/AuthNZ/repos/orgs_teams_repo.py[1729-1753]
tldw_Server_API/app/core/AuthNZ/membership_writer.py[1056-1088]
tldw_Server_API/app/core/AuthNZ/membership_writer.py[1110-1134]
tldw_Server_API/app/services/org_invite_service.py[375-405]
tldw_Server_API/app/api/v1/endpoints/org_invites.py[111-128]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The explicit-team best-effort savepoint catches all exceptions from `MembershipWriter.apply_membership_mutations`, including transient database lock and timeout failures that must reach the invite endpoint's 503 mapper. This can commit organization/default-team membership while silently dropping the requested explicit-team membership.

## Issue Context
Best-effort handling should be limited to expected explicit-team enrollment failures. Infrastructure and transaction-control exceptions must be re-raised after rolling back the savepoint so the outer transaction aborts and the client receives a retryable response. Apply the same correction to both PostgreSQL and SQLite branches.

## Fix Focus Areas
- tldw_Server_API/app/core/AuthNZ/repos/orgs_teams_repo.py[1729-1753]
- tldw_Server_API/app/services/org_invite_service.py[375-405]
- tldw_Server_API/app/api/v1/endpoints/org_invites.py[111-128]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Custom exceptions outside core ✓ Resolved 📘 Rule violation ⌂ Architecture
Description
New/modified custom exception classes are defined outside tldw_Server_API/app/core/exceptions.py.
This violates the requirement to centralize custom exceptions in the core exceptions module.
Code

tldw_Server_API/app/core/AuthNZ/membership_writer.py[R44-45]

+class MembershipWriteError(UserRegistrationException):
+    """Base class for sanitized runtime membership-write failures."""
Evidence
The PR adds multiple exception classes in membership_writer.py (e.g., MembershipWriteError) and
modifies ProviderCredentialAliasConflictError in user_provider_secrets.py, but neither is
located in tldw_Server_API/app/core/exceptions.py as required.

Rule 224217: Centralize custom exceptions in core module
tldw_Server_API/app/core/AuthNZ/membership_writer.py[30-66]
tldw_Server_API/app/core/AuthNZ/user_provider_secrets.py[15-21]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Custom exceptions are being introduced/modified in feature modules (e.g., AuthNZ) instead of being centralized in `tldw_Server_API/app/core/exceptions.py`.

## Issue Context
Centralizing exception types improves consistency and makes it easier for callers to catch/handle domain errors across modules.

## Fix Focus Areas
- tldw_Server_API/app/core/AuthNZ/membership_writer.py[30-70]
- tldw_Server_API/app/core/AuthNZ/user_provider_secrets.py[15-22]
- tldw_Server_API/app/core/exceptions.py[1-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. _manager_write_context missing docstring ✓ Resolved 📘 Rule violation ✧ Quality
Description
The newly added helper _manager_write_context() has no docstring, violating the requirement that
all functions include docstrings. Missing docstrings reduce maintainability and make intent harder
to audit.
Code

tldw_Server_API/app/api/v1/endpoints/shared_keys_scoped.py[R98-101]

+def _manager_write_context(
+    principal: AuthPrincipal,
+) -> ActorMembershipWriteContext:
+    return ActorMembershipWriteContext(
Evidence
The compliance rule requires a docstring as the first statement in every function;
_manager_write_context() immediately returns an ActorMembershipWriteContext without any
docstring string literal.

Rule 224214: Require docstrings for all modules, classes, and functions
tldw_Server_API/app/api/v1/endpoints/shared_keys_scoped.py[98-108]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_manager_write_context()` is a newly added function but has no docstring.

## Issue Context
Compliance requires docstrings for all functions (including helpers) in modified files.

## Fix Focus Areas
- tldw_Server_API/app/api/v1/endpoints/shared_keys_scoped.py[98-108]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. _read_membership builds SQL f-string ✓ Resolved 📘 Rule violation ⛨ Security
Description
membership_writer._read_membership() interpolates table/column names into SQL via f-strings and
suppresses Bandit with # nosec without a justification comment. This violates the requirements to
use parameterized queries, avoid new Bandit findings, and to keep raw SQL confined to
/app/core/DB_Management/.
Code

tldw_Server_API/app/core/AuthNZ/membership_writer.py[R2368-2371]

+            row = await conn.fetchrow(
+                f"SELECT role, status FROM public.{table} "  # nosec B608
+                f"WHERE {scope_column} = $1 AND user_id = $2",
+                scope_id,
Evidence
The new code interpolates SQL identifiers using f-strings (public.{table} and {scope_column})
and uses # nosec B608 with no justification comment, while also placing raw SQL in a module
outside /app/core/DB_Management/.

Rule 224220: Use parameterized queries for all database interactions
Rule 224231: Enforce database access through /app/core/DB_Management/ only
Rule 380645: No new Bandit security findings in changed code
tldw_Server_API/app/core/AuthNZ/membership_writer.py[2364-2378]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`membership_writer._read_membership()` constructs SQL with f-strings (`public.{table}` and `WHERE {scope_column} = ...`) and suppresses Bandit (`# nosec B608`) without an inline justification. This violates compliance requirements for parameterized queries and zero new Bandit findings, and also introduces raw SQL in a non-`/app/core/DB_Management/` module.

## Issue Context
Even if `table`/`scope_column` currently come from a local allow-list, string interpolation in SQL is forbidden by policy and increases the risk of SQL injection if future refactors widen the inputs.

## Fix Focus Areas
- tldw_Server_API/app/core/AuthNZ/membership_writer.py[2364-2380]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 74 rules
Review mode: 🧠 Deep: This is a high-risk auth/authorization and concurrency refactor spanning 83 files, 499 hunks, and multiple independent membership-write paths, making several subtle defects plausibly easy to miss in one pass.

Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread tldw_Server_API/app/core/AuthNZ/membership_writer.py Outdated
Comment thread tldw_Server_API/app/core/AuthNZ/membership_writer.py Outdated
Comment thread tldw_Server_API/app/api/v1/endpoints/shared_keys_scoped.py
Comment thread tldw_Server_API/app/core/AuthNZ/repos/orgs_teams_repo.py Outdated
@rmusser01
rmusser01 force-pushed the codex/userprofiles-stage2-membership-writer branch from a69e888 to 8b49161 Compare September 7, 2026 20:18
@rmusser01
rmusser01 force-pushed the codex/userprofiles-stage2-membership-writer branch from 5a65e44 to 7ee154d Compare September 7, 2026 22:03
@rmusser01
rmusser01 force-pushed the codex/userprofiles-stage2-membership-writer branch from 7ee154d to 9f175b2 Compare September 7, 2026 22:12
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.

1 participant