refactor(db): introduce ContextVar DAO layer and optimize transaction granularity#678
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a72456380
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| async with async_session() as session: | ||
| yield session |
There was a problem hiding this comment.
When no ContextVar transaction is active, this fallback opens a fresh AsyncSession and yields it, but the DAO write helpers only flush() and this context closes without committing. Several refactored callers still already have a request/session but are not wrapped in transaction() (for example BaseAuthProvider._create_new_user now calls find_or_create_identity(...) without passing its db), so first-time SSO/channel identity creation is rolled back before the subsequent User insert references it, leading to missing identities or FK failures. Bind the caller's session into the context or ensure standalone DAO write sessions commit/rollback.
Useful? React with 👍 / 👎.
| """ | ||
| # ── OrgMember binding ──────────────────────────────────────────────────── | ||
|
|
||
| async def bind_org_member(self, user: User) -> None: |
There was a problem hiding this comment.
Changing bind_org_member to accept only user leaves existing tenant join/create paths still calling registration_service.bind_org_member(db, new_user/current_user) in backend/app/api/tenants.py (checked lines 216, 230, 344, and 361). Those endpoints will now raise TypeError: ... takes 2 positional arguments but 3 were given after flushing user changes and before committing whenever a user creates or joins an organization/invitation, so update those call sites or retain a compatible signature.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da0f469b55
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| tenant, | ||
| request: Request | None = None, | ||
| *, | ||
| sso_redirect_enabled: bool = True, |
There was a problem hiding this comment.
With this default the helper no longer reads sso_custom_domain_redirect_enabled; I checked current call sites, and /sso/config, Google Workspace redirects, and enterprise auto-assignment still call get_tenant_sso_base_url(...) without the new argument (backend/app/api/sso.py:113, backend/app/services/google_workspace_oauth.py:82, backend/app/api/enterprise.py:786). In installations where admins disabled custom-domain SSO redirects, those flows will still build tenant-domain/subdomain URLs, so OAuth callbacks can be sent to domains they explicitly disabled.
Useful? React with 👍 / 👎.
| tool_name = fn["name"] | ||
| raw_args = fn.get("arguments", "{}") | ||
| logger.info(f"[LLM] Calling tool: {tool_name}({json.dumps(raw_args, ensure_ascii=False)[:100]})") | ||
| logger.info(f"[LLM] Calling tool: {tool_name}({json.dumps(raw_args, ensure_ascii=False)})") |
There was a problem hiding this comment.
When a tool call includes long or sensitive arguments (for example email bodies/attachments, MCP config secrets, or file contents passed to write/edit tools), this now emits the entire argument JSON to application logs; before this change it was capped at 100 characters. In production that can leak user content or secrets and can bloat logs for large tool calls, so keep truncation or add redaction here.
Useful? React with 👍 / 👎.
合并 main 分支最新变更: - chore(release): cut v1.10.2 (dataelement#679) - refactor(db): ContextVar DAO layer + transaction granularity (dataelement#678) 冲突解决策略: - database/auth_registry/email/platform/password/system_email: 保留 ours (日志前缀) - agents/auth/registration/caller/AgentDetailPage/docker-compose: 采用 theirs (main 最新)
- database.py: 添加 _session_ctx ContextVar + transaction() (main dataelement#678 DAO 层依赖) - caller.py: 添加 _format_friendly_error (lsp4j jsonrpc_router 依赖) - migration add_chat_message_query_index: SQLAlchemy 2.0 兼容 (engine.connect) - docker-compose.yml: 恢复 backend ports 映射 (合并时丢失)
1 participant
💡 Proposed Changes
This PR introduces two major database-related optimizations and a file encoding fix:
AsyncSessionseamlessly usingContextVar(_session_ctx), eliminating the need to explicitly passdb/sessionarguments through registry services, helpers, and handlers.verify_password_async,hash_password_async), external third-party network calls, Redis interactions, and read-only SELECT queries outsideasync with transaction()blocks.🧪 Verification
Verified that all 38 tests in the authentication, onboarding, registry, password reset, and SSO modules pass cleanly.