refactor: bind authenticated user via ContextVar by Aureliolo · Pull Request #1858 · Aureliolo/synthorg

Aureliolo · 2026-05-10T09:37:56Z

Closes #1849.

Replaces the inline get_auth_user_id(request) plumbing with a per-asyncio-Task ContextVar populated by a small AuthContextMiddleware that runs immediately after ApiAuthMiddleware. Controllers and audit helpers now read the authenticated user via no-argument accessors.

Summary

New module src/synthorg/api/auth/context.py exposes:
- _authenticated_user: ContextVar[AuthenticatedUser | None] (module-private)
- get_authenticated_user() -> AuthenticatedUser and get_authenticated_user_id() -> str
- authenticated_user_scope(user) async context manager (token-based set/reset, mirrors cost_recording_scope)
- AuthContextMiddleware (ASGIMiddleware, scopes = (ScopeType.HTTP,)) — token-based reset in try/finally around the inner ASGI dispatch
- AuthContextMissingError(DomainError) with status_code = 500, error_category = INTERNAL, error_code = INTERNAL_ERROR. Reading the var while unset is a server invariant violation, not a client error — surfaces middleware misconfiguration loudly instead of masking it as the prior "api" sentinel.
Wired in src/synthorg/api/middleware_factory.py::_build_middleware immediately after auth_middleware (positions 4 of 9 with rate limits enabled, 3 of 6 without). Middleware ordering documented inline with an explicit INVARIANT: block.
Migrated all 14 get_auth_user_id(request) callsites across 7 controllers (agent_identity_versions.py, agents.py, company.py, departments.py, subworkflows.py, workflow_versions.py, workflows.py). 7 handlers no longer take request: Request (the only use was the auth lookup); 4 keep the parameter because they still call request.headers / publish_ws_event(request, ...).
Migrated request_audit_actor → audit_actor_from_context() (raises AuthContextMissingError when unset). Added BACKGROUND_AUDIT_ACTOR: Final[ProviderAuditActor] constant for explicit opt-in by background paths. 6 callers in providers.py migrated; 3 mock-patches in test_providers.py collapsed into one parametrised loop driven by the real authenticated_user_scope() helper.
Removed API_AUTH_USER_FALLBACK event constant (no remaining callers). Added API_AUTH_CONTEXT_BOUND (debug-level, emitted on bind) and API_AUTH_CONTEXT_SKIPPED (debug-level, emitted when scope["user"] is non-None and not an AuthenticatedUser — surfaces middleware ordering / scope-mutation bugs).
Renamed get_auth_user_id → get_authenticated_user_id (no compat alias; pre-alpha).

Tests

New unit suite tests/unit/api/auth/test_context.py (14 tests):
- Unset accessors raise AuthContextMissingError with the documented default message and 500 status.
- authenticated_user_scope resets the var on normal exit and on exception.
- Nested scopes shadow correctly; outer is restored on exit.
- Child-task inheritance: confirms ContextVar is copied (not shared) into child Tasks; child rebinding does not leak back to parent.
- AuthContextMiddleware binds when scope["user"] is AuthenticatedUser; passes through when missing or wrong type; resets after dispatch on both success and exception paths (the actual reset-contract enforcement; per-task isolation alone would mask a forgotten .reset(token)).
New integration suite tests/integration/api/auth/test_context_request_isolation.py (3 tests):
- 8 concurrent requests parked on threading.Barrier(8); each carries a different user; all return their own id with no overlap, and the var is unset post-test.
- Failure path: half the handlers raise after the barrier; survivors still see their own user; var is unset post-test; tightened to == 500 so an unexpected 5xx code does not pass silently.
- Wiring check: handler on a path with no bound user returns a 500 end-to-end via the global exception handler.
Rewrote tests/unit/api/controllers/test_workflow_helpers.py for the new helpers; uses the public ContextVar accessor instead of MagicMock(request).
Updated middleware-count assertion in tests/unit/api/test_app.py from 8 → 9.

Honest scope note: the integration test is a wiring check; the per-task .reset(token) enforcement is verified by the unit tests.

Pre-PR review

20 agents (15 main + 4 mini-pass + issue-resolution-verifier). 13 returned APPROVE / no findings ≥ 80 confidence. 7 returned 11 actionable items, all implemented per user-approved triage:

Source	Finding	Resolution
docs-consistency	`docs/api/layer.md` Auth section missing context module	Added `::: synthorg.api.auth.context` directive + paragraph
test-quality	Broad `>= 500` assertion	Tightened to `== 500`
mini-pass-race	Barrier could deadlock if handler errors before reaching it	Added explicit safety note in module docstring
type-design	Middleware silently passes through wrong-type `scope.user`	Added `API_AUTH_CONTEXT_SKIPPED` debug log
test-quality	Three monkey-patches of `audit_actor_from_context` are fragile	Replaced with `authenticated_user_scope` driving the real path
type-design	Middleware ordering invariant	Added explicit `INVARIANT:` block
type-design	`authenticated_user_scope()` discoverability	Added background-task usage example to docstring
test-quality	Three near-identical provider validation tests	Collapsed into one parametrised loop
test-quality	Repeated `f"user-{i:02d}"` format	Extracted `_user_id_for()` helper
pr-test-analyzer	Suggested extra tests	Added child-task inheritance + production-path 500
(style)	Module-level test helpers vs fixtures	No change — pytest-idiomatic

issue-resolution-verifier confirmed all 5 acceptance criteria RESOLVED at 95–100% confidence, including the user-approved widening from ContextVar[str | None] → ContextVar[AuthenticatedUser | None] so request_audit_actor could migrate in the same PR.

Test plan

uv run python -m pytest tests/unit/api/auth/ tests/integration/api/auth/ \
    tests/unit/api/controllers/test_workflow_helpers.py \
    tests/unit/api/controllers/test_providers.py \
    tests/unit/api/test_app.py --no-cov -q
uv run mypy src/ tests/
uv run ruff check src/ tests/
uv run pre-commit run --all-files

All green locally on Windows + Python 3.14 (-n 8 --dist=loadfile per project standard).

github-actions · 2026-05-10T09:38:08Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

coderabbitai · 2026-05-10T09:38:11Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b8f53bea-f101-4f3e-91b2-0cdce1cb51d7

📥 Commits

Reviewing files that changed from the base of the PR and between 59da2b9 and fdeac6a.

📒 Files selected for processing (3)

src/synthorg/api/auth/context.py
src/synthorg/observability/events/api.py
tests/unit/api/auth/test_context.py

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

GitHub Check: Build Backend
GitHub Check: CodSpeed Python benchmarks
GitHub Check: Lighthouse Site
GitHub Check: Build Web Assets (melange)
GitHub Check: Type Check
GitHub Check: Test (Python 3.14)
GitHub Check: Build Preview
GitHub Check: Analyze (go)
GitHub Check: Analyze (python)
GitHub Check: Analyze (javascript-typescript)

🧰 Additional context used

📓 Path-based instructions (5)

src/synthorg/!(persistence)/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Only src/synthorg/persistence/ may import sqlite/psycopg or emit raw SQL.

Files:

src/synthorg/api/auth/context.py
src/synthorg/observability/events/api.py

src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Configuration precedence: DB > env > YAML > code default via SettingsService/ConfigResolver; no os.environ.get outside startup.
No hardcoded numeric values except allowlist: 0/1/-1, HTTP codes, hex masks, powers-of-2. Numerics live in settings/definitions/. Enforced by scripts/check_no_magic_numbers.py.
Comments explain WHY only; no reviewer citations, issue back-refs, or migration framing. Enforced by check_no_review_origin_in_code.py and check_no_migration_framing.py.
Do not use from __future__ import annotations (Python 3.14+ has PEP 649). Use PEP 758 except: except A, B: requires parens when binding.
Type hints on public functions; mypy strict. Google-style docstrings. Line length 88; functions <50 lines; files <800 lines.
Define errors as <Domain><Condition>Error inheriting from DomainError; never inherit directly from Exception, RuntimeError, etc. Enforced by check_domain_error_hierarchy.py.
Pydantic v2: frozen + extra="forbid" on API DTOs (Request/Response/Snapshot/Result/Envelope/Status/Info/Summary suffixes); use @computed_field for derived; use NotBlankStr for identifiers.
Use args models at every system boundary; call parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py.
Immutability: use model_copy(update=...) or copy.deepcopy(); deepcopy at system boundaries.
Async: use asyncio.TaskGroup for fan-out/fan-in; helpers catch Exception (re-raise MemoryError/RecursionError).
Clock seam: inject clock: Clock | None = None parameter; tests inject FakeClock. Lifecycle: services own _lifecycle_lock; timed-out stops mark unrestartable.
Untrusted content (SEC-1): use wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML.
Import logger as from synthorg.observability import get_logger; variable always named logger. Never import logging or print() in app code.
Event names from observability.events.<domain> constants; use stru...

Files:

src/synthorg/api/auth/context.py
src/synthorg/observability/events/api.py

{src,tests}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Vendor-agnostic naming: NEVER use real vendor names in project code/tests. Use example-provider, test-provider, example-{large,medium,small}-001. Allowed only in .claude/, third-party imports, providers/presets.py, web/public/provider-logos/.

Files:

src/synthorg/api/auth/context.py
src/synthorg/observability/events/api.py
tests/unit/api/auth/test_context.py

src/**/*.py

⚙️ CodeRabbit configuration file

This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.

Files:

src/synthorg/api/auth/context.py
src/synthorg/observability/events/api.py

tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Test markers: use @pytest.mark.{unit,integration,e2e,slow}. Async tests use auto. Global timeout 30s. Coverage 80% minimum.
xdist: use -n 8 --dist=loadfile (auto-applied via pyproject addopts; loadfile prevents 3.14+ Windows ProactorEventLoop leak).
Windows: unit tests use WindowsSelectorEventLoopPolicy (3.14 IOCP teardown race). Subprocess tests override back.
Mock-spec: every Mock declares spec=ConcreteClass; baseline at scripts/mock_spec_baseline.txt.
FakeClock: import from tests._shared.fake_clock; inject via clock= parameter.
Hypothesis: 10 deterministic CI examples; failures are real bugs (fix + add @example(...)). Never skip/xfail for flaky tests; fix fundamentally.
Use asyncio.Event().wait() instead of sleep(large) to avoid flaky tests.

Files:

tests/unit/api/auth/test_context.py

⚙️ CodeRabbit configuration file

Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare @settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which @given() honors automatically.

Files:

tests/unit/api/auth/test_context.py

🧠 Learnings (1)

📚 Learning: 2026-05-05T09:04:46.195Z

Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.

Applied to files:

src/synthorg/api/auth/context.py
src/synthorg/observability/events/api.py
tests/unit/api/auth/test_context.py

🔇 Additional comments (6)

src/synthorg/observability/events/api.py (1)

20-22: Auth-context event constants are well defined.

The new constants are consistent with the api.auth.* naming scheme and correctly model the bind/skipped/missing paths.

src/synthorg/api/auth/context.py (3)

69-85: Missing-context accessor path is implemented correctly.

Logging API_AUTH_CONTEXT_MISSING immediately before raising AuthContextMissingError gives the required operator breadcrumb for this invariant violation.

98-124: authenticated_user_scope() uses the correct token reset pattern.

The set()/reset() lifecycle is robust for both normal and exceptional exits.

145-190: Middleware lifecycle handling is solid.

Always binding/resetting the ContextVar (including skipped paths) and normalizing invalid scope["user"] to None correctly prevents auth-context leakage and inconsistent downstream auth state.

tests/unit/api/auth/test_context.py (2)

39-147: Core ContextVar accessor and scope tests are comprehensive.

These cases cover the primary invariants (unset behavior, logging path, nested scoping, and task inheritance) effectively.

149-335: Middleware behavior tests are strong and targeted.

The suite validates dispatch binding, exception-path cleanup, inherited-context isolation, and scope["user"] normalization, which gives good confidence in the new middleware contract.

Walkthrough

This PR centralizes authenticated-user access via a per-task ContextVar: it adds synthorg.api.auth.context (ContextVar, accessors, AuthContextMissingError, authenticated_user_scope, AuthContextMiddleware), re-exports them at package level, inserts the middleware after auth_middleware, replaces request-threaded helpers with context-based audit helpers, updates controller signatures to drop Request parameters, adds unit/integration tests for isolation and lifecycle, and updates docs and test baseline entries.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 36.71% which is insufficient. The required threshold is 40.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'refactor: bind authenticated user via ContextVar' clearly and concisely summarizes the main change—replacing request-threaded auth plumbing with a ContextVar-backed approach.
Description check	✅ Passed	The description is thorough and related to the changeset, detailing new modules, migrations, test coverage, and closure of issue `#1849`.
Linked Issues check	✅ Passed	The PR fully meets all five acceptance criteria from issue `#1849`: introduces ContextVar[AuthenticatedUser \| None], implements middleware set/reset with token-based reset, removes 14 get_auth_user_id callsites, includes comprehensive unit/integration tests, and maintains mypy strict compliance.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the objectives: new auth.context module, middleware integration, controller migrations to use ContextVar accessors, test updates, and observability event updates—no extraneous changes detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codspeed-hq · 2026-05-10T09:40:54Z

Merging this PR will not alter performance

✅ 33 untouched benchmarks
⏩ 21 skipped benchmarks¹

_{Comparing refactor/issue-1849-auth-contextvar (fdeac6a) with main (e90550e)}

21 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/synthorg/api/auth/context.py`:
- Around line 146-170: The early-return branch that handles
non-AuthenticatedUser requests (scope_user not instance of AuthenticatedUser)
currently bypasses ContextVar management and can leak a previously bound user;
modify that branch to explicitly bind a "cleared" token before calling next_app
and always reset it afterwards: call _authenticated_user.set(None) (capturing
the token) before awaiting next_app in the skipped-path and ensure you call
_authenticated_user.reset(token) in a finally block so
get_authenticated_user*/audit_actor_from_context will not observe a stale
principal or require AuthContextMissingError to be raised; keep the existing
debug logging (API_AUTH_CONTEXT_SKIPPED) and use the same next_app/receive/send
flow.

In `@tests/unit/api/test_app.py`:
- Around line 1261-1272: Update the test_middleware_stack_has_nine_entries test
(which calls _build_middleware) to assert the actual middleware entries and
ordering rather than only length: verify that the list contains the expected
classes (e.g., ip_floor, ETagMiddleware, auth_middleware, AuthContextMiddleware,
csrf_mw, unauth_rl, RequestLoggingMiddleware, auth_rl,
PerOpConcurrencyMiddleware) and specifically assert that AuthContextMiddleware
appears immediately after auth_middleware in the returned mw list; use
class/type or repr/name checks on elements of mw to lock the positions and fail
if AuthContextMiddleware is missing or in the wrong place.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 594f92e7-6c30-4dae-98e1-79fe29e0b7e4

📥 Commits

Reviewing files that changed from the base of the PR and between db6650b and f8f28d7.

📒 Files selected for processing (21)

docs/api/layer.md
scripts/mock_spec_baseline.txt
src/synthorg/api/auth/__init__.py
src/synthorg/api/auth/context.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/controllers/agent_identity_versions.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/company.py
src/synthorg/api/controllers/departments.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/workflows.py
src/synthorg/api/middleware_factory.py
src/synthorg/observability/events/api.py
tests/integration/api/auth/__init__.py
tests/integration/api/auth/test_context_request_isolation.py
tests/unit/api/auth/test_context.py
tests/unit/api/controllers/test_providers.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/test_app.py

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

GitHub Check: Build Backend
GitHub Check: CodSpeed Python benchmarks
GitHub Check: Lighthouse Site
GitHub Check: Type Check
GitHub Check: Test (Python 3.14)
GitHub Check: Build Web Assets (melange)
GitHub Check: Build Preview
GitHub Check: Analyze (go)
GitHub Check: Analyze (javascript-typescript)
GitHub Check: Analyze (python)

🧰 Additional context used

📓 Path-based instructions (6)

src/synthorg/!(persistence)/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Only src/synthorg/persistence/ may import sqlite/psycopg or emit raw SQL.

Files:

src/synthorg/observability/events/api.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/controllers/departments.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/auth/__init__.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/agent_identity_versions.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/workflows.py

src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Configuration precedence: DB > env > YAML > code default via SettingsService/ConfigResolver; no os.environ.get outside startup.
No hardcoded numeric values except allowlist: 0/1/-1, HTTP codes, hex masks, powers-of-2. Numerics live in settings/definitions/. Enforced by scripts/check_no_magic_numbers.py.
Comments explain WHY only; no reviewer citations, issue back-refs, or migration framing. Enforced by check_no_review_origin_in_code.py and check_no_migration_framing.py.
Do not use from __future__ import annotations (Python 3.14+ has PEP 649). Use PEP 758 except: except A, B: requires parens when binding.
Type hints on public functions; mypy strict. Google-style docstrings. Line length 88; functions <50 lines; files <800 lines.
Define errors as <Domain><Condition>Error inheriting from DomainError; never inherit directly from Exception, RuntimeError, etc. Enforced by check_domain_error_hierarchy.py.
Pydantic v2: frozen + extra="forbid" on API DTOs (Request/Response/Snapshot/Result/Envelope/Status/Info/Summary suffixes); use @computed_field for derived; use NotBlankStr for identifiers.
Use args models at every system boundary; call parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py.
Immutability: use model_copy(update=...) or copy.deepcopy(); deepcopy at system boundaries.
Async: use asyncio.TaskGroup for fan-out/fan-in; helpers catch Exception (re-raise MemoryError/RecursionError).
Clock seam: inject clock: Clock | None = None parameter; tests inject FakeClock. Lifecycle: services own _lifecycle_lock; timed-out stops mark unrestartable.
Untrusted content (SEC-1): use wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML.
Import logger as from synthorg.observability import get_logger; variable always named logger. Never import logging or print() in app code.
Event names from observability.events.<domain> constants; use stru...

Files:

src/synthorg/observability/events/api.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/controllers/departments.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/auth/__init__.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/agent_identity_versions.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/workflows.py

{src,tests}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Vendor-agnostic naming: NEVER use real vendor names in project code/tests. Use example-provider, test-provider, example-{large,medium,small}-001. Allowed only in .claude/, third-party imports, providers/presets.py, web/public/provider-logos/.

Files:

src/synthorg/observability/events/api.py
tests/unit/api/test_app.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/controllers/departments.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/auth/__init__.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/agent_identity_versions.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/auth/test_context.py
tests/integration/api/auth/test_context_request_isolation.py
src/synthorg/api/controllers/agents.py
tests/unit/api/controllers/test_providers.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/workflows.py

src/**/*.py

⚙️ CodeRabbit configuration file

This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.

Files:

src/synthorg/observability/events/api.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/controllers/departments.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/auth/__init__.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/agent_identity_versions.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/workflows.py

tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Test markers: use @pytest.mark.{unit,integration,e2e,slow}. Async tests use auto. Global timeout 30s. Coverage 80% minimum.
xdist: use -n 8 --dist=loadfile (auto-applied via pyproject addopts; loadfile prevents 3.14+ Windows ProactorEventLoop leak).
Windows: unit tests use WindowsSelectorEventLoopPolicy (3.14 IOCP teardown race). Subprocess tests override back.
Mock-spec: every Mock declares spec=ConcreteClass; baseline at scripts/mock_spec_baseline.txt.
FakeClock: import from tests._shared.fake_clock; inject via clock= parameter.
Hypothesis: 10 deterministic CI examples; failures are real bugs (fix + add @example(...)). Never skip/xfail for flaky tests; fix fundamentally.
Use asyncio.Event().wait() instead of sleep(large) to avoid flaky tests.

Files:

tests/unit/api/test_app.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/auth/test_context.py
tests/integration/api/auth/test_context_request_isolation.py
tests/unit/api/controllers/test_providers.py

⚙️ CodeRabbit configuration file

Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare @settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which @given() honors automatically.

Files:

tests/unit/api/test_app.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/auth/test_context.py
tests/integration/api/auth/test_context_request_isolation.py
tests/unit/api/controllers/test_providers.py

docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

docs/**/*.md: Numerics in README and public docs sourced from data/runtime_stats.yaml via  markers.
Web dashboard uses D2 for architecture/nested containers, mermaid for flowcharts/sequence/pipelines. Use Markdown tables for tabular data. D2 theme 200 (Dark Mauve), D2 CLI v0.7.1 pinned in CI.

Files:

docs/api/layer.md

🧠 Learnings (1)

📚 Learning: 2026-05-05T09:04:46.195Z

Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.

Applied to files:

src/synthorg/observability/events/api.py
tests/unit/api/test_app.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/controllers/departments.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/auth/__init__.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/agent_identity_versions.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/auth/test_context.py
tests/integration/api/auth/test_context_request_isolation.py
src/synthorg/api/controllers/agents.py
tests/unit/api/controllers/test_providers.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/workflows.py

🔇 Additional comments (15)

src/synthorg/api/controllers/departments.py (1)

11-11: LGTM!

The import of get_authenticated_user_id and its usage across create_department, update_department, and delete_department correctly replaces the request-based auth extraction. The request parameter is appropriately retained for WebSocket event publishing and header access.

Also applies to: 468-468, 510-510, 557-557

src/synthorg/api/controllers/company.py (1)

9-9: LGTM!

The context-based get_authenticated_user_id() is correctly imported and used in both update_company and reorder_departments. The request parameter is appropriately retained for WebSocket publishing and ETag header access.

Also applies to: 103-103, 146-146

tests/unit/api/controllers/test_workflow_helpers.py (1)

34-57: LGTM!

The test suite correctly validates:

audit_actor_from_context() returns the bound user's id and label within the auth scope

AuthContextMissingError is raised when no user is bound

BACKGROUND_AUDIT_ACTOR sentinel uses "api" for both id and label

The actor object assigned inside the context manager remains valid after scope exit since it's a plain object, not a ContextVar reference.

docs/api/layer.md (1)

55-69: LGTM!

The documentation accurately describes the new AuthContextMiddleware placement, the per-asyncio-Task ContextVar binding mechanism, and the no-argument accessor functions. The API reference for synthorg.api.auth.context is correctly added.

src/synthorg/api/controllers/workflow_versions.py (1)

5-11: LGTM!

The rollback_workflow method correctly removes the unused request parameter and imports are cleaned up (Any, Request removed). The get_authenticated_user_id() accessor properly provides the rollback actor identity for the saved_by parameter.

Also applies to: 319-335

src/synthorg/api/controllers/subworkflows.py (1)

10-18: LGTM!

The create_subworkflow method correctly removes the unused request parameter, with corresponding import cleanup. The get_authenticated_user_id() accessor properly provides the creator identity for the workflow definition.

Also applies to: 301-308

src/synthorg/api/controllers/agent_identity_versions.py (1)

4-11: LGTM!

The rollback_identity method correctly removes the unused request parameter, with corresponding import cleanup. The get_authenticated_user_id() accessor properly provides the actor identity for the rollback rationale message.

Also applies to: 263-298

tests/integration/api/auth/test_context_request_isolation.py (4)

31-35: Appropriate use of private ContextVar for test verification.

Importing _authenticated_user directly is acceptable here—the test needs to verify the ContextVar is properly reset after requests complete. This internal access is appropriate for integration tests that validate middleware reset contracts.

115-153: LGTM!

The concurrent isolation test is well-designed:

Uses threading.Barrier with bounded timeout to ensure all 8 requests are simultaneously in-flight

asyncio.to_thread(barrier.wait, ...) correctly bridges the sync barrier into async handlers

Verifies each request observes its own user ID without cross-request bleed

Post-test assertion confirms the ContextVar is reset to None

155-203: LGTM!

The failure-path isolation test correctly verifies that:

Exceptions in half the handlers don't poison the ContextVar for other concurrent requests

Successful requests (odd-numbered) still observe their own user IDs

Failed requests return HTTP 500 (not 502/503)

The ContextVar is reset to None after all requests complete

205-222: LGTM!

The unauthenticated path test validates the end-to-end behavior when no user is bound: AuthContextMissingError surfaces as HTTP 500 through the real exception handler chain, and the ContextVar remains unset afterward.

src/synthorg/api/controllers/workflows.py (1)

14-14: Auth context migration is consistently applied across workflow mutations.

The write paths now resolve actor identity via get_authenticated_user_id() in a uniform way, matching the request-scoped ContextVar design and removing request-plumbing cleanly.

Also applies to: 173-173, 243-243, 337-337, 383-383

tests/unit/api/auth/test_context.py (1)

39-67: Excellent invariant coverage for ContextVar and middleware lifecycle.

These tests exercise the right failure and isolation paths (unset access, nested scopes, task inheritance, and reset-on-exception), which materially reduces leak/regression risk in the new auth binding model.

Also applies to: 70-125, 191-245

src/synthorg/api/controllers/providers.py (1)

20-20: Audit actor sourcing is consistently migrated to request context.

The affected mutation endpoints now uniformly use audit_actor_from_context(), which aligns with the middleware-bound auth model and removes fragile per-handler request plumbing.

Also applies to: 1029-1029, 1083-1083, 1147-1147, 1242-1242, 1302-1302, 1387-1387

tests/unit/api/controllers/test_providers.py (1)

760-825: Great consolidation: same sanitization guarantee, closer to real execution path.

This looped test keeps coverage for all three capability mutations while validating through authenticated_user_scope, so it catches regressions in both sanitization and context-bound actor wiring.

gemini-code-assist

Code Review

This pull request introduces request-scoped user binding via ContextVar, enabling controllers and helpers to access authenticated user data without explicitly passing Request arguments. It adds AuthContextMiddleware to the API stack and refactors numerous controllers to use the new get_authenticated_user_id and audit_actor_from_context helpers. Feedback suggests changing a log level from debug to warning when the middleware encounters unexpected user types in the scope to improve the observability of configuration bugs.

gemini-code-assist · 2026-05-10T09:43:25Z

+            if scope_user is not None:
+                logger.debug(
+                    API_AUTH_CONTEXT_SKIPPED,
+                    scope_user_type=type(scope_user).__name__,
+                    path=scope.get("path", ""),
+                )


When scope["user"] is present but not an AuthenticatedUser, it indicates a middleware ordering issue or a downstream mutation that breaks the auth invariant. While the handler will eventually raise a 500 error, logging this as a warning rather than debug would improve observability of these configuration bugs in production environments.

Suggested change

if scope_user is not None:

logger.debug(

API_AUTH_CONTEXT_SKIPPED,

scope_user_type=type(scope_user).__name__,

path=scope.get("path", ""),

)

if scope_user is not None:

logger.warning(

API_AUTH_CONTEXT_SKIPPED,

scope_user_type=type(scope_user).__name__,

path=scope.get("path", ""),

)

codecov · 2026-05-10T09:48:54Z

Codecov Report

❌ Patch coverage is 96.77419% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.89%. Comparing base (e90550e) to head (fdeac6a).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/synthorg/api/controllers/providers.py	66.66%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1858   +/-   ##
=======================================
  Coverage   84.88%   84.89%           
=======================================
  Files        1804     1806    +2     
  Lines      105034   105078   +44     
  Branches     9199     9200    +1     
=======================================
+ Hits        89161    89204   +43     
  Misses      13647    13647           
- Partials     2226     2227    +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Pre-reviewed by 20 agents (15 main + 4 mini-pass + issue-resolution). 13 returned APPROVE / no findings; 7 returned 11 actionable items, all implemented (per user-approved triage). - docs/api/layer.md: document new synthorg.api.auth.context module. - AuthContextMiddleware: log API_AUTH_CONTEXT_SKIPPED at debug when scope.user is non-None and not an AuthenticatedUser, so middleware ordering bugs / scope mutations become observable. - middleware_factory.py: explicit INVARIANT comment on AuthContextMiddleware ordering. - authenticated_user_scope(): docstring example showing background-task usage so future maintainers see the right pattern. - test_context_request_isolation.py: tighten 5xx assertion to == 500; extract _user_id_for() helper to keep header values, _make_user factory, and assertions in lockstep; add deadlock-safety note on threading.Barrier; add wiring test that calls get_authenticated_user_id() on an unauthenticated path and asserts the typed 500 surfaces end to end. - test_context.py: add child-task inheritance test confirming ContextVar propagation semantics. - test_providers.py: collapse 3 near-identical capability-mutation validation tests into one parametrised loop driven by the real authenticated_user_scope() helper rather than fragile monkey-patches.

Round 1 review fixes: - src/synthorg/api/auth/context.py: AuthContextMiddleware now binds a token on every code path (None on the skipped branch) so an inherited outer ContextVar can never leak a stale principal into the inner dispatch (CodeRabbit C1). - src/synthorg/api/auth/context.py: API_AUTH_CONTEXT_SKIPPED logged at WARNING instead of debug; the present-but-not-AuthenticatedUser branch indicates middleware misconfiguration and should surface in production logs (Gemini). - tests/unit/api/test_app.py: middleware-stack test asserts mw[3] is AuthContextMiddleware, locking the position so reordering breaks the test instead of silently regressing get_authenticated_user_id() in CSRF / rate-limit / logging layers (CodeRabbit C2). - tests/unit/api/auth/test_context.py: two new regression tests assert the skipped-path no longer leaks an inherited binding, for both the missing-user and wrong-type variants. - tests/unit/api/test_app.py: bare MagicMock() in TestAuthIdentifierForRequest replaced with spec=Request / spec=AuthenticatedUser, satisfying the no-new-bare-mock pre-commit gate now that the file is touched.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/synthorg/api/auth/context.py`:
- Around line 155-171: The branch that detects a non-AuthenticatedUser in
scope_user warns but leaves scope["user"] unchanged which leads to inconsistent
auth state; update the branch inside the else (where API_AUTH_CONTEXT_SKIPPED is
logged) to also normalize the request scope by setting scope["user"] = None (or
raise) so downstream code reading scope["user"] sees the same unauthenticated
state as get_authenticated_user*(); ensure this change is applied alongside the
existing bound_user = None and _authenticated_user.set(bound_user) logic (refer
to symbols scope_user, scope, bound_user, API_AUTH_CONTEXT_SKIPPED, and
_authenticated_user.set).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e907a3b2-8c1a-4b87-b2db-9cdcfc7cdb2d

📥 Commits

Reviewing files that changed from the base of the PR and between db60cad and 762d3a7.

📒 Files selected for processing (21)

docs/api/layer.md
scripts/mock_spec_baseline.txt
src/synthorg/api/auth/__init__.py
src/synthorg/api/auth/context.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/controllers/agent_identity_versions.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/company.py
src/synthorg/api/controllers/departments.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/workflows.py
src/synthorg/api/middleware_factory.py
src/synthorg/observability/events/api.py
tests/integration/api/auth/__init__.py
tests/integration/api/auth/test_context_request_isolation.py
tests/unit/api/auth/test_context.py
tests/unit/api/controllers/test_providers.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/test_app.py

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

GitHub Check: Deploy Preview
GitHub Check: Build Backend
GitHub Check: Build Web Assets (melange)
GitHub Check: Test (Python 3.14)
GitHub Check: CodSpeed Python benchmarks
GitHub Check: Lighthouse Site
GitHub Check: Analyze (python)
GitHub Check: Analyze (javascript-typescript)

🧰 Additional context used

📓 Path-based instructions (6)

src/synthorg/!(persistence)/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Only src/synthorg/persistence/ may import sqlite/psycopg or emit raw SQL.

Files:

src/synthorg/api/auth/__init__.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/controllers/workflows.py
src/synthorg/api/controllers/departments.py
src/synthorg/observability/events/api.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/agent_identity_versions.py

src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Configuration precedence: DB > env > YAML > code default via SettingsService/ConfigResolver; no os.environ.get outside startup.
No hardcoded numeric values except allowlist: 0/1/-1, HTTP codes, hex masks, powers-of-2. Numerics live in settings/definitions/. Enforced by scripts/check_no_magic_numbers.py.
Comments explain WHY only; no reviewer citations, issue back-refs, or migration framing. Enforced by check_no_review_origin_in_code.py and check_no_migration_framing.py.
Do not use from __future__ import annotations (Python 3.14+ has PEP 649). Use PEP 758 except: except A, B: requires parens when binding.
Type hints on public functions; mypy strict. Google-style docstrings. Line length 88; functions <50 lines; files <800 lines.
Define errors as <Domain><Condition>Error inheriting from DomainError; never inherit directly from Exception, RuntimeError, etc. Enforced by check_domain_error_hierarchy.py.
Pydantic v2: frozen + extra="forbid" on API DTOs (Request/Response/Snapshot/Result/Envelope/Status/Info/Summary suffixes); use @computed_field for derived; use NotBlankStr for identifiers.
Use args models at every system boundary; call parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py.
Immutability: use model_copy(update=...) or copy.deepcopy(); deepcopy at system boundaries.
Async: use asyncio.TaskGroup for fan-out/fan-in; helpers catch Exception (re-raise MemoryError/RecursionError).
Clock seam: inject clock: Clock | None = None parameter; tests inject FakeClock. Lifecycle: services own _lifecycle_lock; timed-out stops mark unrestartable.
Untrusted content (SEC-1): use wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML.
Import logger as from synthorg.observability import get_logger; variable always named logger. Never import logging or print() in app code.
Event names from observability.events.<domain> constants; use stru...

Files:

src/synthorg/api/auth/__init__.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/controllers/workflows.py
src/synthorg/api/controllers/departments.py
src/synthorg/observability/events/api.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/agent_identity_versions.py

{src,tests}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Vendor-agnostic naming: NEVER use real vendor names in project code/tests. Use example-provider, test-provider, example-{large,medium,small}-001. Allowed only in .claude/, third-party imports, providers/presets.py, web/public/provider-logos/.

Files:

src/synthorg/api/auth/__init__.py
tests/unit/api/auth/test_context.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/controllers/workflows.py
src/synthorg/api/controllers/departments.py
tests/unit/api/test_app.py
src/synthorg/observability/events/api.py
src/synthorg/api/controllers/workflow_versions.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/controllers/test_providers.py
tests/integration/api/auth/test_context_request_isolation.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/agent_identity_versions.py

src/**/*.py

⚙️ CodeRabbit configuration file

This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.

Files:

src/synthorg/api/auth/__init__.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/controllers/workflows.py
src/synthorg/api/controllers/departments.py
src/synthorg/observability/events/api.py
src/synthorg/api/controllers/workflow_versions.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/agent_identity_versions.py

docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

docs/**/*.md: Numerics in README and public docs sourced from data/runtime_stats.yaml via  markers.
Web dashboard uses D2 for architecture/nested containers, mermaid for flowcharts/sequence/pipelines. Use Markdown tables for tabular data. D2 theme 200 (Dark Mauve), D2 CLI v0.7.1 pinned in CI.

Files:

docs/api/layer.md

tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Test markers: use @pytest.mark.{unit,integration,e2e,slow}. Async tests use auto. Global timeout 30s. Coverage 80% minimum.
xdist: use -n 8 --dist=loadfile (auto-applied via pyproject addopts; loadfile prevents 3.14+ Windows ProactorEventLoop leak).
Windows: unit tests use WindowsSelectorEventLoopPolicy (3.14 IOCP teardown race). Subprocess tests override back.
Mock-spec: every Mock declares spec=ConcreteClass; baseline at scripts/mock_spec_baseline.txt.
FakeClock: import from tests._shared.fake_clock; inject via clock= parameter.
Hypothesis: 10 deterministic CI examples; failures are real bugs (fix + add @example(...)). Never skip/xfail for flaky tests; fix fundamentally.
Use asyncio.Event().wait() instead of sleep(large) to avoid flaky tests.

Files:

tests/unit/api/auth/test_context.py
tests/unit/api/test_app.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/controllers/test_providers.py
tests/integration/api/auth/test_context_request_isolation.py

⚙️ CodeRabbit configuration file

Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare @settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which @given() honors automatically.

Files:

tests/unit/api/auth/test_context.py
tests/unit/api/test_app.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/controllers/test_providers.py
tests/integration/api/auth/test_context_request_isolation.py

🧠 Learnings (1)

📚 Learning: 2026-05-05T09:04:46.195Z

Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.

Applied to files:

src/synthorg/api/auth/__init__.py
tests/unit/api/auth/test_context.py
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/api/controllers/company.py
src/synthorg/api/auth/context.py
src/synthorg/api/middleware_factory.py
src/synthorg/api/controllers/agents.py
src/synthorg/api/controllers/subworkflows.py
src/synthorg/api/controllers/workflows.py
src/synthorg/api/controllers/departments.py
tests/unit/api/test_app.py
src/synthorg/observability/events/api.py
src/synthorg/api/controllers/workflow_versions.py
tests/unit/api/controllers/test_workflow_helpers.py
tests/unit/api/controllers/test_providers.py
tests/integration/api/auth/test_context_request_isolation.py
src/synthorg/api/controllers/providers.py
src/synthorg/api/controllers/agent_identity_versions.py

🔇 Additional comments (15)

src/synthorg/api/controllers/company.py (1)

9-9: LGTM!

The migration from get_auth_user_id(request) to get_authenticated_user_id() is correct. The request parameter is retained appropriately for WebSocket event publishing via publish_ws_event.

Also applies to: 103-103, 146-146

src/synthorg/api/controllers/agent_identity_versions.py (1)

263-268: LGTM!

The rollback_identity handler correctly removes the request parameter and switches to get_authenticated_user_id() for actor derivation. The PEP 758 exception syntax at line 332 is valid for Python 3.14+.

Also applies to: 298-298

src/synthorg/api/controllers/subworkflows.py (1)

301-308: LGTM!

The create_subworkflow handler correctly removes the request parameter since it was only used for get_auth_user_id. The creator is now properly derived from auth context.

tests/unit/api/test_app.py (2)

1261-1279: LGTM!

The test correctly asserts both the middleware stack length (9 entries) and the fixed position of AuthContextMiddleware at index 3. The comment clearly documents the ordering invariant and why it matters for downstream middleware.

1357-1399: LGTM!

The mock specifications are now properly typed with spec=Request and spec=AuthenticatedUser, aligning with the coding guidelines requiring spec=ConcreteClass on all mocks.

src/synthorg/api/controllers/_workflow_helpers.py (1)

19-48: LGTM!

Clean design: audit_actor_from_context() derives the actor from bound auth context and raises on misconfiguration, while BACKGROUND_AUDIT_ACTOR provides an explicit opt-in sentinel for background paths. The docstring clearly documents when to use each.

docs/api/layer.md (1)

55-69: LGTM!

Documentation accurately describes the AuthContextMiddleware placement and the context-based accessor pattern. The API reference entry for synthorg.api.auth.context completes the module documentation.

tests/unit/api/controllers/test_workflow_helpers.py (1)

34-57: LGTM!

Tests properly cover both audit_actor_from_context() (bound user returns correct actor, unbound raises AuthContextMissingError) and the BACKGROUND_AUDIT_ACTOR sentinel values.

tests/integration/api/auth/test_context_request_isolation.py (3)

31-35: Accessing module-private _authenticated_user is acceptable here.

The test imports the module-private _authenticated_user ContextVar directly to verify post-test cleanup (lines 153, 203, 222). This is appropriate for integration tests that need to assert on internal state reset behavior.

115-148: LGTM!

Well-designed concurrency test using threading.Barrier to ensure all 8 requests are simultaneously in-flight before any returns. The pattern correctly catches wiring errors where the ContextVar might be shared across requests.

155-203: LGTM!

Good coverage of the failure path: verifies that exceptions in some handlers don't poison the ContextVar for concurrent successful requests. The raise_server_exceptions=False usage is appropriate for capturing 500 responses without aborting.

src/synthorg/api/controllers/workflows.py (1)

14-14: Auth context migration in write handlers looks correct.

Using get_authenticated_user_id() directly in the mutated write paths keeps actor resolution consistent and removes request-plumbing cleanly without changing endpoint behavior.

Also applies to: 167-173, 237-243, 316-337, 377-383

tests/unit/api/auth/test_context.py (1)

70-125: Excellent isolation/lifecycle coverage for ContextVar behavior.

The new cases exercise the critical reset and non-leak guarantees (nested scopes, child-task inheritance, skipped middleware paths, and exception paths) needed for this refactor.

Also applies to: 191-280

src/synthorg/api/controllers/providers.py (1)

20-20: Context-based audit actor wiring is consistently applied.

The updated mutation handlers correctly derive actor via audit_actor_from_context() and pass it through to management/service calls where required.

Also applies to: 1007-1035, 1061-1089, 1124-1153, 1220-1263, 1286-1318, 1366-1393

tests/unit/api/controllers/test_providers.py (1)

760-825: Nice consolidation of capability-mutation sanitization coverage.

Running these three handlers under authenticated_user_scope validates both the new auth-context dependency and error-message sanitization in one focused, maintainable test.

AuthContextMiddleware now normalises scope["user"] to None on the wiring-bug branch (present-but-not-AuthenticatedUser). Without this, downstream layers that read scope["user"] directly (rate-limit identifiers, the anonymous-tier gate, request-logging) would observe a foreign principal as 'authenticated' while get_authenticated_user*() would raise AuthContextMissingError -- inconsistent auth state across the request. Adds a regression test that captures scope["user"] from inside the inner dispatch and asserts both the captured value and the post-handle scope are None on the wrong-type branch (CodeRabbit).

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/synthorg/api/auth/context.py`:
- Around line 75-77: The code in the auth context getter
(_authenticated_user.get() -> raise AuthContextMissingError) should log a
structured WARNING/ERROR event before raising so operators have a breadcrumb;
update the branch where user is None to call the observability event constant
(observability.events.<domain> e.g. observability.events.auth or similar) via
the module logger (logger.warning or logger.error) with the event constant and
structured kwargs (e.g. logger.warning(EVENT, user=None, reason="missing auth
context", function="get_authenticated_user")) and then raise
AuthContextMissingError; reference the symbols _authenticated_user,
AuthContextMissingError, and the module logger/observability.events constants to
locate where to add this log.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b115d70f-213d-4fbc-ad57-8a4e3546cf2d

📥 Commits

Reviewing files that changed from the base of the PR and between 762d3a7 and 59da2b9.

📒 Files selected for processing (2)

src/synthorg/api/auth/context.py
tests/unit/api/auth/test_context.py

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

GitHub Check: Deploy Preview
GitHub Check: Build Backend
GitHub Check: Build Web Assets (melange)
GitHub Check: CodSpeed Python benchmarks
GitHub Check: Lighthouse Site
GitHub Check: Test (Python 3.14)
GitHub Check: Analyze (python)
GitHub Check: Analyze (javascript-typescript)

🧰 Additional context used

📓 Path-based instructions (5)

src/synthorg/!(persistence)/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Only src/synthorg/persistence/ may import sqlite/psycopg or emit raw SQL.

Files:

src/synthorg/api/auth/context.py

src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Configuration precedence: DB > env > YAML > code default via SettingsService/ConfigResolver; no os.environ.get outside startup.
No hardcoded numeric values except allowlist: 0/1/-1, HTTP codes, hex masks, powers-of-2. Numerics live in settings/definitions/. Enforced by scripts/check_no_magic_numbers.py.
Comments explain WHY only; no reviewer citations, issue back-refs, or migration framing. Enforced by check_no_review_origin_in_code.py and check_no_migration_framing.py.
Do not use from __future__ import annotations (Python 3.14+ has PEP 649). Use PEP 758 except: except A, B: requires parens when binding.
Type hints on public functions; mypy strict. Google-style docstrings. Line length 88; functions <50 lines; files <800 lines.
Define errors as <Domain><Condition>Error inheriting from DomainError; never inherit directly from Exception, RuntimeError, etc. Enforced by check_domain_error_hierarchy.py.
Pydantic v2: frozen + extra="forbid" on API DTOs (Request/Response/Snapshot/Result/Envelope/Status/Info/Summary suffixes); use @computed_field for derived; use NotBlankStr for identifiers.
Use args models at every system boundary; call parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py.
Immutability: use model_copy(update=...) or copy.deepcopy(); deepcopy at system boundaries.
Async: use asyncio.TaskGroup for fan-out/fan-in; helpers catch Exception (re-raise MemoryError/RecursionError).
Clock seam: inject clock: Clock | None = None parameter; tests inject FakeClock. Lifecycle: services own _lifecycle_lock; timed-out stops mark unrestartable.
Untrusted content (SEC-1): use wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML.
Import logger as from synthorg.observability import get_logger; variable always named logger. Never import logging or print() in app code.
Event names from observability.events.<domain> constants; use stru...

Files:

src/synthorg/api/auth/context.py

{src,tests}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Vendor-agnostic naming: NEVER use real vendor names in project code/tests. Use example-provider, test-provider, example-{large,medium,small}-001. Allowed only in .claude/, third-party imports, providers/presets.py, web/public/provider-logos/.

Files:

src/synthorg/api/auth/context.py
tests/unit/api/auth/test_context.py

src/**/*.py

⚙️ CodeRabbit configuration file

This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.

Files:

src/synthorg/api/auth/context.py

tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Test markers: use @pytest.mark.{unit,integration,e2e,slow}. Async tests use auto. Global timeout 30s. Coverage 80% minimum.
xdist: use -n 8 --dist=loadfile (auto-applied via pyproject addopts; loadfile prevents 3.14+ Windows ProactorEventLoop leak).
Windows: unit tests use WindowsSelectorEventLoopPolicy (3.14 IOCP teardown race). Subprocess tests override back.
Mock-spec: every Mock declares spec=ConcreteClass; baseline at scripts/mock_spec_baseline.txt.
FakeClock: import from tests._shared.fake_clock; inject via clock= parameter.
Hypothesis: 10 deterministic CI examples; failures are real bugs (fix + add @example(...)). Never skip/xfail for flaky tests; fix fundamentally.
Use asyncio.Event().wait() instead of sleep(large) to avoid flaky tests.

Files:

tests/unit/api/auth/test_context.py

⚙️ CodeRabbit configuration file

Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare @settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which @given() honors automatically.

Files:

tests/unit/api/auth/test_context.py

🧠 Learnings (1)

📚 Learning: 2026-05-05T09:04:46.195Z

Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.

Applied to files:

src/synthorg/api/auth/context.py
tests/unit/api/auth/test_context.py

🔇 Additional comments (1)

tests/unit/api/auth/test_context.py (1)

246-312: Nice coverage on the skipped-path regressions.

These cases pin both inherited-context clearing and scope["user"] normalization, which are the easiest middleware invariants to accidentally break later.

get_authenticated_user() now logs a structured WARNING (API_AUTH_CONTEXT_MISSING / api.auth.context_missing) before raising AuthContextMissingError, per the project's 'error paths log WARNING/ERROR with context before raising' convention. Without the breadcrumb, an operator triaging a 500 from a wiring-bug call site (excluded path misrouted to a request-coupled helper, AuthContextMiddleware missing, helper invoked outside a request) would see only the exception envelope and have nothing to grep. Added a test that asserts the warning fires before the raise (CodeRabbit).

## Highlights > _AI-generated summary (model: `openai/gpt-4.1-mini` via GitHub Models). Commit-based changelog below._ ### What you'll notice - Password and secret fields now include an eye-toggle for easier visibility control. - Containers running without probes are shown as healthy in the doctor command. - Unloaded and missing PR-review agents are restored and available again. ### What's new - Gate baseline protection is enhanced to block em-dashes during writing. ### Under the hood - Replaced Atlas with yoyo-migrations for persistence management. - Refactored codebase extensively, including context-bound user authentication and registry pattern for enums. - Improved linting by draining magic number usages and tightening mock and constant checks. - Updated CI to retry Docker pushes on network timeout errors. - Updated apko lockfiles for dependency management.  :robot: I have created a release *beep* *boop* --- ## [0.8.3](v0.8.2...v0.8.3) (2026-05-12) ### Features * harden gate baseline protection + block em-dashes at write time ([#1860](#1860)) ([b41f151](b41f151)) * **web:** eye-toggle on every password / secret field ([#1873](#1873)) ([9070387](9070387)) ### Bug Fixes * **ci:** retry Docker push on Go net/http deadline + cancellation errors ([#1877](#1877)) ([23a0bfa](23a0bfa)) * **cli:** render running-no-probe containers as healthy in doctor ([#1870](#1870)) ([6263795](6263795)) * restore unloaded and missing PR-review agents ([#1875](#1875)) ([db004fd](db004fd)), closes [#1871](#1871) ### Refactoring * bind authenticated user via ContextVar ([#1858](#1858)) ([57ed0b4](57ed0b4)) * code-structure cleanup (sub-tasks D + F + G + H + I) ([#1859](#1859)) ([362e5c8](362e5c8)) * convert enum dispatch to registry pattern ([#1854](#1854)) ([e90550e](e90550e)) * drain no_magic_numbers baseline to zero via Final hoists ([#1856](#1856) phase 2) ([#1872](#1872)) ([ec8109e](ec8109e)) * drain pagination + loop-init + kill-switch baselines ([#1857](#1857)) ([#1868](#1868)) ([115c3c2](115c3c2)) * **persistence:** replace Atlas with yoyo-migrations ([#1876](#1876)) ([1b7e975](1b7e975)), closes [#1874](#1874) * protocols audit follow-up (REVIEW + fold pass) ([#1869](#1869)) ([af33ddb](af33ddb)) * protocols audit follow-up REMOVE pass ([#1867](#1867)) ([dd1eebc](dd1eebc)) * tighten check_mock_spec gate, add mock_of[T], drain baseline ([#1862](#1862)) ([240a253](240a253)) * tighten check_no_magic_numbers for named module constants ([#1856](#1856)) ([#1866](#1866)) ([90c933b](90c933b)) ### CI/CD * update apko lockfiles ([#1863](#1863)) ([2bd32e6](2bd32e6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: synthorg-repo-bot[bot] <279117679+synthorg-repo-bot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Aureliolo temporarily deployed to atlas May 10, 2026 09:38 — with GitHub Actions Inactive

Aureliolo temporarily deployed to cloudflare-preview May 10, 2026 09:39 — with GitHub Actions Inactive

coderabbitai Bot requested changes May 10, 2026

View reviewed changes

Comment thread src/synthorg/api/auth/context.py

Comment thread tests/unit/api/test_app.py

gemini-code-assist Bot reviewed May 10, 2026

View reviewed changes

Aureliolo temporarily deployed to atlas May 10, 2026 10:00 — with GitHub Actions Inactive

Aureliolo temporarily deployed to cloudflare-preview May 10, 2026 10:01 — with GitHub Actions Inactive

coderabbitai Bot approved these changes May 10, 2026

View reviewed changes

Aureliolo force-pushed the refactor/issue-1849-auth-contextvar branch from d21c167 to db60cad Compare May 10, 2026 10:25

Aureliolo temporarily deployed to atlas May 10, 2026 10:25 — with GitHub Actions Inactive

Aureliolo temporarily deployed to cloudflare-preview May 10, 2026 10:27 — with GitHub Actions Inactive

Aureliolo added 3 commits May 10, 2026 12:43

refactor: bind authenticated user via ContextVar (#1849)

70d5811

Aureliolo force-pushed the refactor/issue-1849-auth-contextvar branch from db60cad to 762d3a7 Compare May 10, 2026 10:46

Aureliolo temporarily deployed to atlas May 10, 2026 10:46 — with GitHub Actions Inactive

Aureliolo temporarily deployed to cloudflare-preview May 10, 2026 10:47 — with GitHub Actions Inactive

coderabbitai Bot requested changes May 10, 2026

View reviewed changes

Comment thread src/synthorg/api/auth/context.py

Aureliolo temporarily deployed to atlas May 10, 2026 11:08 — with GitHub Actions Inactive

Aureliolo temporarily deployed to cloudflare-preview May 10, 2026 11:10 — with GitHub Actions Inactive

coderabbitai Bot requested changes May 10, 2026

View reviewed changes

Comment thread src/synthorg/api/auth/context.py

Aureliolo temporarily deployed to atlas May 10, 2026 11:34 — with GitHub Actions Inactive

Aureliolo temporarily deployed to cloudflare-preview May 10, 2026 11:35 — with GitHub Actions Inactive

coderabbitai Bot approved these changes May 10, 2026

View reviewed changes

Aureliolo merged commit 57ed0b4 into main May 10, 2026
77 checks passed

Aureliolo deleted the refactor/issue-1849-auth-contextvar branch May 10, 2026 11:51

Aureliolo temporarily deployed to cloudflare-preview May 10, 2026 11:51 — with GitHub Actions Inactive

synthorg-repo-bot Bot mentioned this pull request May 10, 2026

chore(main): release 0.8.3 #1861

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: bind authenticated user via ContextVar#1858

refactor: bind authenticated user via ContextVar#1858
Aureliolo merged 5 commits into
mainfrom
refactor/issue-1849-auth-contextvar

Aureliolo commented May 10, 2026

Uh oh!

github-actions Bot commented May 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 10, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

codspeed-hq Bot commented May 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 10, 2026

Uh oh!

codecov Bot commented May 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Aureliolo commented May 10, 2026

Summary

Tests

Pre-PR review

Test plan

Uh oh!

github-actions Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Scanned Files

Uh oh!

coderabbitai Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

codspeed-hq Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 10, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented May 10, 2026 •

edited

Loading

coderabbitai Bot commented May 10, 2026 •

edited

Loading

codspeed-hq Bot commented May 10, 2026 •

edited

Loading

codecov Bot commented May 10, 2026 •

edited

Loading